Ticket25762 034 01 #50
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
No behavior change, just to make it easier to find callbacks and for the sake of our human brain to parse the list properly. Signed-off-by: David Goulet <dgoulet@torproject.org>
In tor, we have a series of possible "roles" that the tor daemon can be enabled for. They are: Client, Bridge, Relay, Authority (directory or bridge) and Onion service. They can be combined sometimes. For instance, a Directory Authority is also a Relay. This adds a "roles" field to a periodic event item object which is used to know for which roles the event is for. The next step is to enable the event only if the roles apply. No behavior change at this commit. Pars of #25762 Signed-off-by: David Goulet <dgoulet@torproject.org>
This is the next step where we only setup the events for the roles we are configured for. This commit builds but won't work because we can setup twice the same events because the roles can be combined. Next commit will add a fix to guard against that since now it is a reality. Signed-off-by: David Goulet <dgoulet@torproject.org>
When we set up a periodic event, the mainloop event object is created. Use this to know if the event is already set up and if so, we don't set it up again. We also only schedule or launche a periodic event if that object is set. Signed-off-by: David Goulet <dgoulet@torproject.org>
Useful for us to know which callbacks were enabled. Signed-off-by: David Goulet <dgoulet@torproject.org>
In case we transitionned to a new role in Tor, we need to launch the right periodic events for it. Signed-off-by: David Goulet <dgoulet@torproject.org>
nmathewson
reviewed
Apr 17, 2018
src/or/main.c
Outdated
| @@ -1458,6 +1458,55 @@ initialize_periodic_events_cb(evutil_socket_t fd, short events, void *data) | |||
| } | |||
| } | |||
|
|
|||
| /* Helper function: Set up periodic_events[] that only matches the given | |||
There was a problem hiding this comment.
Documentation should be a doxygen comment, and should explain what "matches" means.
src/or/main.c
Outdated
| for (int i = 0; periodic_events[i].name; ++i) { | ||
| periodic_event_item_t *item = &periodic_events[i]; | ||
| if (item->roles & roles) { | ||
| periodic_event_setup(item); |
There was a problem hiding this comment.
How does this work when we call this function later on? If we're going to do this, we need to make periodic_event_setup idempotent.
Actually, we can safely just call periodic_event_setup() on all of these events, and use periodic_event_launch to start the ones we really want. We can add a new periodic_event_disable() to turn off the ones we don't want.
| const or_options_t *options = get_options(); | ||
|
|
||
| /* At this stage, we are booting up so we need to know for which roles this | ||
| * tor instance has. */ |
There was a problem hiding this comment.
Let's make just one bitfield here, and call setup_periodic_events_by_roles once?
| * currently enabled events and trying to destroy only the one that could be | ||
| * affected. */ | ||
| if (periodic_events_initialized) { | ||
| teardown_periodic_events(); |
There was a problem hiding this comment.
This is not okay -- if we tear them down and then set them up again, the actual timers will all be reset.
|
This was merged with another PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some stuff are still missing but this is a first pass at this. See ticket on trac for details.