Make periodic events array with flags including when they are enabled/disabled

This is part of #25500 (moved) bigger ticket.

The goal here is to put the periodic events (not the per-second callbacks) into an array of objects where an object has flags and enable/disable switch.

So far, the flags would indicate conditions on when to run the callbacks. One of them would be "do not run if disable network is on".

The object also needs to tell for which "tor entity" the callbacks applies. For instance, a certain callback would only apply to Client. Or a callback is only for Dirauth.

Probably more (or less) things will be added to the object as we start implementing.

changed milestone to %Tor: 0.3.4.x-final

added 034-roadmap-subtask client component::core tor/tor milestone::Tor: 0.3.4.x-final owner::dgoulet parent::25500 priority::medium resolution::implemented reviewer::nickm s8-perf severity::normal sponsor::8 status::closed type::defect labels

Normalize keywords.

Trac:
Keywords: performance deleted, s8-perf added

Alpha version: ticket25762_034_01.

This can probably be unit tested (at least for the double setup checks). Oh and it doesn't have yet "flags" to tell the setup process to run it or not like the "disable network". I'll do this once we agree on the interface.

PR (not for merge): https://github.com/torproject/tor/pull/50

I left some comments on the PR.

My main suggestion here would be:

• Always initialize every periodic event. Instead of initializing only some...
• Only call periodic_event_launch() on the events that should be running.
• Add a periodic_event_disable() that delegates to mainloop_event_cancel(); call that on the events that should not be running.

David asked me to look at his revised ticket25762_034_02. Looking better! I've left some suggested changes on my branch of the same name.

Ok branch ticket25762_034_03 has the latest from Nickm with couple of tweaks from me including a fixup commit.

So imo, we are happy with the interface, this simply needs unit tests.

This needs unit tests, and I also want to review these remaining things that I didn't check when I was reviewing the initial branch:

• Did we accidentally "lose" any periodic callbacks?
• Are all of the flags set correctly?

Also:

• We should decide how this approach will handle events that turn off when DisableNetwork is set.

Replying to nickm:

This needs unit tests, and I also want to review these remaining things that I didn't check when I was reviewing the initial branch:

• Did we accidentally "lose" any periodic callbacks?

Yah... I think this means someone needs to go over commit f9370609e49e72d0 and make sure I didn't forget one :S.

• Are all of the flags set correctly?

Indeed. Same commit from above. It is possible an event applies to a role I failed to notice...

Also:

• We should decide how this approach will handle events that turn off when DisableNetwork is set.

I think we can do this part of #25376 (moved) and not within this branch.

Ok unit tests added. I've rebased on latest master, merge fixup commits, cleaned up the commit list to make them nicer.

Branch: ticket25762_034_04 PR: https://github.com/torproject/tor/pull/56

Trac:
Reviewer: N/A to nickm
Status: assigned to needs_review

I had a couple of questions on some of the flags, but otherwise this looks good.

Once you've looked at the above questions, and made changes as appropriate, two final tests to do, if you haven't already:

• Does it regress make test-network-all?
• Does it cause any problems when you change flags on a running Tor instance and send it a SIGHUP?

Trac:
Status: needs_review to needs_revision

Replying to nickm:

• Does it regress make test-network-all?

It did! but I pushed a fixup commit.

• Does it cause any problems when you change flags on a running Tor instance and send it a SIGHUP?

Looks fine with my testing!

The _04 branch has the fixup and the _05 branch has been rebased and squashed on latest master.

Trac:
Status: needs_revision to needs_review

great; I've merged the _05 branch to master!

Trac:
Status: needs_review to closed
Resolution: N/A to implemented

closed

mentioned in issue #27212 (moved)

moved to tpo/core/tor#25762 (closed)