From a7cfddc8d18c39be8fb212ee2a96da2d1905d9c8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 30 Oct 2019 09:49:55 -0400 Subject: Make a new structure for tracking subsystem status. We used to have only one boolean per subsystem, but we're about to have a little more information. --- src/app/main/subsysmgr.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) (limited to 'src/app/main/subsysmgr.c') diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index 1f4bc840f2..5fc298dbf6 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -31,12 +31,22 @@ **/ static bool subsystem_array_validated = false; +/** + * Runtime status of a single subsystem. + **/ +typedef struct subsys_status_t { + /** True if the given subsystem is initialized. */ + bool initialized; +} subsys_status_t; + +/** An overestimate of the number of subsystems. */ +#define N_SYS_STATUS 128 /** * True if a given subsystem is initialized. Expand this array if there * are more than this number of subsystems. (We'd rather not * dynamically allocate in this module.) **/ -static bool sys_initialized[128]; +static subsys_status_t sys_status[N_SYS_STATUS]; /** * Exit with a raw assertion if the subsystems list is inconsistent; @@ -48,8 +58,8 @@ check_and_setup(void) if (subsystem_array_validated) return; - raw_assert(ARRAY_LENGTH(sys_initialized) >= n_tor_subsystems); - memset(sys_initialized, 0, sizeof(sys_initialized)); + raw_assert(ARRAY_LENGTH(sys_status) >= n_tor_subsystems); + memset(sys_status, 0, sizeof(sys_status)); int last_level = MIN_SUBSYS_LEVEL; @@ -97,7 +107,7 @@ subsystems_init_upto(int target_level) continue; if (sys->level > target_level) break; - if (sys_initialized[i]) + if (sys_status[i].initialized) continue; int r = 0; if (sys->initialize) { @@ -112,7 +122,7 @@ subsystems_init_upto(int target_level) sys->name, i); raw_assert_unreached_msg("A subsystem couldn't be initialized."); } - sys_initialized[i] = true; + sys_status[i].initialized = true; } return 0; @@ -132,7 +142,7 @@ subsystems_add_pubsub_upto(pubsub_builder_t *builder, continue; if (sys->level > target_level) break; - if (! sys_initialized[i]) + if (! sys_status[i].initialized) continue; int r = 0; if (sys->add_pubsub) { @@ -186,13 +196,13 @@ subsystems_shutdown_downto(int target_level) continue; if (sys->level <= target_level) break; - if (! sys_initialized[i]) + if (! sys_status[i].initialized) continue; if (sys->shutdown) { log_debug(LD_GENERAL, "Shutting down %s", sys->name); sys->shutdown(); } - sys_initialized[i] = false; + sys_status[i].initialized = false; } } @@ -208,7 +218,7 @@ subsystems_prefork(void) const subsys_fns_t *sys = tor_subsystems[i]; if (!sys->supported) continue; - if (! sys_initialized[i]) + if (! sys_status[i].initialized) continue; if (sys->prefork) { log_debug(LD_GENERAL, "Pre-fork: %s", sys->name); @@ -229,7 +239,7 @@ subsystems_postfork(void) const subsys_fns_t *sys = tor_subsystems[i]; if (!sys->supported) continue; - if (! sys_initialized[i]) + if (! sys_status[i].initialized) continue; if (sys->postfork) { log_debug(LD_GENERAL, "Post-fork: %s", sys->name); @@ -250,7 +260,7 @@ subsystems_thread_cleanup(void) const subsys_fns_t *sys = tor_subsystems[i]; if (!sys->supported) continue; - if (! sys_initialized[i]) + if (! sys_status[i].initialized) continue; if (sys->thread_cleanup) { log_debug(LD_GENERAL, "Thread cleanup: %s", sys->name); -- cgit v1.2.3-54-g00ecf From 7ac4f9d5ec1b31a0d4b76ab62c1afc039c8fe627 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 30 Oct 2019 09:28:34 -0400 Subject: Give subsystems optional config formats and state formats. The formats, when provided, are now added to the global config_mgr_t objects. --- src/app/config/config.c | 2 ++ src/app/config/statefile.c | 3 ++ src/app/main/subsysmgr.c | 68 +++++++++++++++++++++++++++++++++++++++++++++- src/app/main/subsysmgr.h | 4 +++ src/lib/subsys/subsys.h | 13 +++++++++ 5 files changed, 89 insertions(+), 1 deletion(-) (limited to 'src/app/main/subsysmgr.c') diff --git a/src/app/config/config.c b/src/app/config/config.c index 5f9a55ed17..7317a5d2f1 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -918,6 +918,8 @@ get_options_mgr(void) { if (PREDICT_UNLIKELY(options_mgr == NULL)) { options_mgr = config_mgr_new(&options_format); + int rv = subsystems_register_options_formats(options_mgr); + tor_assert(rv == 0); config_mgr_freeze(options_mgr); } return options_mgr; diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c index db4d780a78..f6b0915da4 100644 --- a/src/app/config/statefile.c +++ b/src/app/config/statefile.c @@ -45,6 +45,7 @@ #include "feature/relay/routermode.h" #include "lib/sandbox/sandbox.h" #include "app/config/statefile.h" +#include "app/main/subsysmgr.h" #include "lib/encoding/confline.h" #include "lib/net/resolve.h" #include "lib/version/torversion.h" @@ -180,6 +181,8 @@ get_state_mgr(void) { if (PREDICT_UNLIKELY(state_mgr == NULL)) { state_mgr = config_mgr_new(&state_format); + int rv = subsystems_register_state_formats(state_mgr); + tor_assert(rv == 0); config_mgr_freeze(state_mgr); } return state_mgr; diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index 5fc298dbf6..e5c76b8e55 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -14,10 +14,12 @@ #include "orconfig.h" #include "app/main/subsysmgr.h" +#include "lib/confmgt/confmgt.h" #include "lib/dispatch/dispatch_naming.h" #include "lib/dispatch/msgtypes.h" #include "lib/err/torerr.h" #include "lib/log/log.h" +#include "lib/log/util_bug.h" #include "lib/malloc/malloc.h" #include "lib/pubsub/pubsub_build.h" #include "lib/pubsub/pubsub_connect.h" @@ -37,6 +39,10 @@ static bool subsystem_array_validated = false; typedef struct subsys_status_t { /** True if the given subsystem is initialized. */ bool initialized; + /** Index for this subsystem's options object, or -1 for none. */ + int options_idx; + /** Index for this subsystem's state object, or -1 for none. */ + int state_idx; } subsys_status_t; /** An overestimate of the number of subsystems. */ @@ -48,6 +54,18 @@ typedef struct subsys_status_t { **/ static subsys_status_t sys_status[N_SYS_STATUS]; +/** Set status to a default (not set-up) state. */ +static void +subsys_status_clear(subsys_status_t *status) +{ + if (!status) + return; + memset(status, 0, sizeof(*status)); + status->initialized = false; + status->state_idx = -1; + status->options_idx = -1; +} + /** * Exit with a raw assertion if the subsystems list is inconsistent; * initialize the subsystem_initialized array. @@ -77,6 +95,8 @@ check_and_setup(void) sys->name, i, sys->level, last_level); raw_assert_unreached_msg("There is a bug in subsystem_list.c"); } + subsys_status_clear(&sys_status[i]); + last_level = sys->level; } @@ -202,7 +222,7 @@ subsystems_shutdown_downto(int target_level) log_debug(LD_GENERAL, "Shutting down %s", sys->name); sys->shutdown(); } - sys_status[i].initialized = false; + subsys_status_clear(&sys_status[i]); } } @@ -268,3 +288,49 @@ subsystems_thread_cleanup(void) } } } + +/** + * Register all subsystem-declared options formats in mgr. + * + * Return 0 on success, -1 on failure. + **/ +int +subsystems_register_options_formats(config_mgr_t *mgr) +{ + tor_assert(mgr); + check_and_setup(); + + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + if (sys->options_format) { + int options_idx = config_mgr_add_format(mgr, sys->options_format); + sys_status[i].options_idx = options_idx; + log_debug(LD_CONFIG, "Added options format for %s with index %d", + sys->name, options_idx); + } + } + return 0; +} + +/** + * Register all subsystem-declared state formats in mgr. + * + * Return 0 on success, -1 on failure. + **/ +int +subsystems_register_state_formats(config_mgr_t *mgr) +{ + tor_assert(mgr); + check_and_setup(); + + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + if (sys->state_format) { + int state_idx = config_mgr_add_format(mgr, sys->state_format); + sys_status[i].state_idx = state_idx; + log_debug(LD_CONFIG, "Added state format for %s with index %d", + sys->name, state_idx); + } + } + return 0; +} diff --git a/src/app/main/subsysmgr.h b/src/app/main/subsysmgr.h index f8bc83e0ad..54193f9649 100644 --- a/src/app/main/subsysmgr.h +++ b/src/app/main/subsysmgr.h @@ -31,4 +31,8 @@ void subsystems_prefork(void); void subsystems_postfork(void); void subsystems_thread_cleanup(void); +struct config_mgr_t; +int subsystems_register_options_formats(struct config_mgr_t *mgr); +int subsystems_register_state_formats(struct config_mgr_t *mgr); + #endif /* !defined(TOR_SUBSYSMGR_T) */ diff --git a/src/lib/subsys/subsys.h b/src/lib/subsys/subsys.h index 91abdb7d74..1cb3fe94a5 100644 --- a/src/lib/subsys/subsys.h +++ b/src/lib/subsys/subsys.h @@ -14,6 +14,7 @@ #include struct pubsub_connector_t; +struct config_format_t; /** * A subsystem is a part of Tor that is initialized, shut down, configured, @@ -88,6 +89,18 @@ typedef struct subsys_fns_t { **/ void (*shutdown)(void); + /** + * A config_format_t describing all of the torrc fields owned by this + * subsystem. + **/ + const struct config_format_t *options_format; + + /** + * A config_format_t describing all of the DataDir/state fields owned by + * this subsystem. + **/ + const struct config_format_t *state_format; + } subsys_fns_t; /** -- cgit v1.2.3-54-g00ecf From 52c0ab4af3ec152c4b78669acf8877ca27d66097 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 30 Oct 2019 10:29:43 -0400 Subject: Add subsys functions for receiving/flushing states and options. These functions are in the subsystem, not in the config_format_t, since they are about how the format is _used_, not about _what it is_. --- src/app/config/config.c | 3 +- src/app/config/statefile.c | 4 +++ src/app/main/subsysmgr.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++ src/app/main/subsysmgr.h | 8 +++++ src/lib/subsys/subsys.h | 29 ++++++++++++++++++ 5 files changed, 119 insertions(+), 1 deletion(-) (limited to 'src/app/main/subsysmgr.c') diff --git a/src/app/config/config.c b/src/app/config/config.c index 7317a5d2f1..8929349709 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -987,7 +987,8 @@ set_options(or_options_t *new_val, char **msg) global_options = old_options; return -1; } - if (options_act(old_options) < 0) { /* acting on the options failed. die. */ + if (subsystems_set_options(get_options_mgr(), new_val) < 0 || + options_act(old_options) < 0) { /* acting on the options failed. die. */ if (! tor_event_loop_shutdown_is_pending()) { log_err(LD_BUG, "Acting on config options left us in a broken state. Dying."); diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c index f6b0915da4..5fdb15ace1 100644 --- a/src/app/config/statefile.c +++ b/src/app/config/statefile.c @@ -316,6 +316,9 @@ or_state_set(or_state_t *new_state) tor_assert(new_state); config_free(get_state_mgr(), global_state); global_state = new_state; + if (subsystems_set_state(get_state_mgr(), global_state) < 0) { + ret = -1; + } if (entry_guards_parse_state(global_state, 1, &err)<0) { log_warn(LD_GENERAL,"%s",err); tor_free(err); @@ -519,6 +522,7 @@ or_state_save(time_t now) /* Call everything else that might dirty the state even more, in order * to avoid redundant writes. */ + (void) subsystems_flush_state(get_state_mgr(), global_state); entry_guards_update_state(global_state); rep_hist_update_state(global_state); circuit_build_times_update_state(get_circuit_build_times(), global_state); diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index e5c76b8e55..bd9af11444 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -334,3 +334,79 @@ subsystems_register_state_formats(config_mgr_t *mgr) } return 0; } + +/** + * Call all appropriate set_options() methods to tell the various subsystems + * about a new set of torrc options. Return 0 on success, -1 on + * nonrecoverable failure. + **/ +int +subsystems_set_options(const config_mgr_t *mgr, struct or_options_t *options) +{ + /* XXXX This does not yet handle reversible option assignment; I'll + * do that later in this branch. */ + + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + if (sys_status[i].options_idx >= 0 && sys->set_options) { + void *obj = config_mgr_get_obj_mutable(mgr, options, + sys_status[i].options_idx); + int rv = sys->set_options(obj); + if (rv < 0) { + log_err(LD_CONFIG, "Error when handling option for %s; " + "cannot proceed.", sys->name); + return -1; + } + } + } + return 0; +} + +/** + * Call all appropriate set_state() methods to tell the various subsystems + * about an initial DataDir/state file. Return 0 on success, -1 on + * nonrecoverable failure. + **/ +int +subsystems_set_state(const config_mgr_t *mgr, struct or_state_t *state) +{ + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + if (sys_status[i].state_idx >= 0 && sys->set_state) { + void *obj = config_mgr_get_obj_mutable(mgr, state, + sys_status[i].state_idx); + int rv = sys->set_state(obj); + if (rv < 0) { + log_err(LD_CONFIG, "Error when handling state for %s; " + "cannot proceed.", sys->name); + return -1; + } + } + } + return 0; +} + +/** + * Call all appropriate flush_state() methods to tell the various subsystems + * to update the state objects in state. Return 0 on success, + * -1 on failure. + **/ +int +subsystems_flush_state(const config_mgr_t *mgr, struct or_state_t *state) +{ + int result = 0; + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + if (sys_status[i].state_idx >= 0 && sys->flush_state) { + void *obj = config_mgr_get_obj_mutable(mgr, state, + sys_status[i].state_idx); + int rv = sys->flush_state(obj); + if (rv < 0) { + log_warn(LD_CONFIG, "Error when flushing state to state object for %s", + sys->name); + result = -1; + } + } + } + return result; +} diff --git a/src/app/main/subsysmgr.h b/src/app/main/subsysmgr.h index 54193f9649..7e5fe76362 100644 --- a/src/app/main/subsysmgr.h +++ b/src/app/main/subsysmgr.h @@ -34,5 +34,13 @@ void subsystems_thread_cleanup(void); struct config_mgr_t; int subsystems_register_options_formats(struct config_mgr_t *mgr); int subsystems_register_state_formats(struct config_mgr_t *mgr); +struct or_options_t; +struct or_state_t; +int subsystems_set_options(const struct config_mgr_t *mgr, + struct or_options_t *options); +int subsystems_set_state(const struct config_mgr_t *mgr, + struct or_state_t *state); +int subsystems_flush_state(const struct config_mgr_t *mgr, + struct or_state_t *state); #endif /* !defined(TOR_SUBSYSMGR_T) */ diff --git a/src/lib/subsys/subsys.h b/src/lib/subsys/subsys.h index 1cb3fe94a5..29a90c049d 100644 --- a/src/lib/subsys/subsys.h +++ b/src/lib/subsys/subsys.h @@ -101,6 +101,35 @@ typedef struct subsys_fns_t { **/ const struct config_format_t *state_format; + /** + * Receive an options object as defined by options_format. Return 0 + * on success, -1 on failure. + * + * It is safe to store the pointer to the object until set_options() + * is called again. */ + int (*set_options)(void *); + + /* XXXX Add an implementation for options_act_reversible() later in this + * branch. */ + + /** + * Receive a state object as defined by state_format. Return 0 on success, + * -1 on failure. + * + * It is safe to store the pointer to the object; set_state() is only + * called on startup. + **/ + int (*set_state)(void *); + + /** + * Update any information that needs to be stored in the provided state + * object (as defined by state_format). Return 0 on success, -1 on failure. + * + * The object provided here will be the same one as provided earlier to + * set_state(). This method is called when we are about to save the state + * to disk. + **/ + int (*flush_state)(void *); } subsys_fns_t; /** -- cgit v1.2.3-54-g00ecf From b06e9d8ad58bfe11d2c1f6a921ba059658a578f2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 1 Nov 2019 10:41:02 -0400 Subject: Add testing-only functions to get the subsystem config/state indices --- src/app/main/subsysmgr.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ src/app/main/subsysmgr.h | 5 +++++ 2 files changed, 53 insertions(+) (limited to 'src/app/main/subsysmgr.c') diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index bd9af11444..4189519cad 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -335,6 +335,54 @@ subsystems_register_state_formats(config_mgr_t *mgr) return 0; } +#ifdef TOR_UNIT_TESTS +/** + * Helper: look up the index for sys. Return -1 if the subsystem + * is not recognized. + **/ +static int +subsys_get_idx(const subsys_fns_t *sys) +{ + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + if (sys == tor_subsystems[i]) + return (int)i; + } + return -1; +} + +/** + * Return the current state-manager's index for any state held by the + * subsystem sys. If sys has no options, return -1. + * + * Using raw indices can be error-prone: only do this from the unit + * tests. If you need a way to access another subsystem's configuration, + * that subsystem should provide access functions. + **/ +int +subsystems_get_options_idx(const subsys_fns_t *sys) +{ + int i = subsys_get_idx(sys); + tor_assert(i >= 0); + return sys_status[i].options_idx; +} + +/** + * Return the current state-manager's index for any state held by the + * subsystem sys. If sys has no state, return -1. + * + * Using raw indices can be error-prone: only do this from the unit + * tests. If you need a way to access another subsystem's state + * that subsystem should provide access functions. + **/ +int +subsystems_get_state_idx(const subsys_fns_t *sys) +{ + int i = subsys_get_idx(sys); + tor_assert(i >= 0); + return sys_status[i].state_idx; +} +#endif + /** * Call all appropriate set_options() methods to tell the various subsystems * about a new set of torrc options. Return 0 on success, -1 on diff --git a/src/app/main/subsysmgr.h b/src/app/main/subsysmgr.h index 7e5fe76362..c1138e1ff3 100644 --- a/src/app/main/subsysmgr.h +++ b/src/app/main/subsysmgr.h @@ -43,4 +43,9 @@ int subsystems_set_state(const struct config_mgr_t *mgr, int subsystems_flush_state(const struct config_mgr_t *mgr, struct or_state_t *state); +#ifdef TOR_UNIT_TESTS +int subsystems_get_options_idx(const subsys_fns_t *sys); +int subsystems_get_state_idx(const subsys_fns_t *sys); +#endif + #endif /* !defined(TOR_SUBSYSMGR_T) */ -- cgit v1.2.3-54-g00ecf From 3afbb29bee060b191e10aaec134a819047c3cf5e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 5 Nov 2019 10:24:34 -0500 Subject: subsysmgr: use IDX_NONE is an exception value, not -1. --- src/app/main/subsysmgr.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'src/app/main/subsysmgr.c') diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index 4189519cad..8be4a7d75c 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -33,15 +33,19 @@ **/ static bool subsystem_array_validated = false; +/** Index value indicating that a subsystem has no options/state object, and + * so that object does not have an index. */ +#define IDX_NONE (-1) + /** * Runtime status of a single subsystem. **/ typedef struct subsys_status_t { /** True if the given subsystem is initialized. */ bool initialized; - /** Index for this subsystem's options object, or -1 for none. */ + /** Index for this subsystem's options object, or IDX_NONE for none. */ int options_idx; - /** Index for this subsystem's state object, or -1 for none. */ + /** Index for this subsystem's state object, or IDX_NONE for none. */ int state_idx; } subsys_status_t; @@ -62,8 +66,8 @@ subsys_status_clear(subsys_status_t *status) return; memset(status, 0, sizeof(*status)); status->initialized = false; - status->state_idx = -1; - status->options_idx = -1; + status->state_idx = IDX_NONE; + status->options_idx = IDX_NONE; } /** -- cgit v1.2.3-54-g00ecf