commit a7681525ab670c2b7a783f9e1285bf9a8e97d1ea Author: Nick Mathewson nickm@torproject.org Date: Tue Jan 15 10:46:01 2019 -0500

Add function to clear publish bindings.

When we clean up, we'd like to clear all the bindings that refer to a dispatch_t, so that they don't have dangling pointers to it. --- src/lib/pubsub/pubsub_build.c | 30 +++++++++++++++++++++++++----- src/lib/pubsub/pubsub_build.h | 20 +++++++++++++++----- src/lib/pubsub/pubsub_builder_st.h | 6 +++--- src/test/test_pubsub_msg.c | 2 +- 4 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/src/lib/pubsub/pubsub_build.c b/src/lib/pubsub/pubsub_build.c index 7e4ab5ba8..64cdcc1d5 100644 --- a/src/lib/pubsub/pubsub_build.c +++ b/src/lib/pubsub/pubsub_build.c @@ -242,10 +242,10 @@ pubsub_connector_define_type_(pubsub_connector_t *con, * for <b>d</b>. */ static void -dispatch_fill_pub_binding_backptrs(pubsub_builder_t *builder, - dispatch_t *d) +pubsub_items_install_bindings(pubsub_items_t *items, + dispatch_t *d) { - SMARTLIST_FOREACH_BEGIN(builder->items->items, pubsub_cfg_t *, cfg) { + SMARTLIST_FOREACH_BEGIN(items->items, pubsub_cfg_t *, cfg) { if (cfg->pub_binding) { // XXXX we could skip this for STUB publishers, and for any publishers // XXXX where all subscribers are STUB. @@ -255,12 +255,28 @@ dispatch_fill_pub_binding_backptrs(pubsub_builder_t *builder, }

/** + * Remove the dispatch_ptr fields for all the relevant publish bindings + * in <b>items</b>. The prevents subsequent dispatch_pub_() calls from + * sending messages to a dispatcher that has been freed. + **/ +void +pubsub_items_clear_bindings(pubsub_items_t *items) +{ + SMARTLIST_FOREACH_BEGIN(items->items, pubsub_cfg_t *, cfg) { + if (cfg->pub_binding) { + cfg->pub_binding->dispatch_ptr = NULL; + } + } SMARTLIST_FOREACH_END(cfg); +} + +/** * Create a new dispatcher as configured in a pubsub_builder_t. * * Consumes and frees its input. **/ dispatch_t * -pubsub_builder_finalize(pubsub_builder_t *builder) +pubsub_builder_finalize(pubsub_builder_t *builder, + pubsub_items_t **items_out) { dispatch_t *dispatcher = NULL; tor_assert_nonfatal(builder->n_connectors == 0); @@ -276,7 +292,11 @@ pubsub_builder_finalize(pubsub_builder_t *builder) if (!dispatcher) goto err;

- dispatch_fill_pub_binding_backptrs(builder, dispatcher); + pubsub_items_install_bindings(builder->items, dispatcher); + if (items_out) { + *items_out = builder->items; + builder->items = NULL; /* Prevent free */ + }

err: pubsub_builder_free(builder); diff --git a/src/lib/pubsub/pubsub_build.h b/src/lib/pubsub/pubsub_build.h index 199eab219..d2920e021 100644 --- a/src/lib/pubsub/pubsub_build.h +++ b/src/lib/pubsub/pubsub_build.h @@ -33,6 +33,13 @@ typedef struct pubsub_builder_t pubsub_builder_t; typedef struct pubsub_connector_t pubsub_connector_t;

/** + * A "pubsub items" holds the configuration items used to configure a + * pubsub_builder. After the builder is finalized, this field is extracted, + * and used later to tear down pointers that enable publishing. + **/ +typedef struct pubsub_items_t pubsub_items_t; + +/** * Create a new pubsub_builder. This should only happen in the * main-init code. */ @@ -75,13 +82,16 @@ void pubsub_connector_free_(pubsub_connector_t *); * This should happen after every subsystem has initialized, and before * entering the mainloop. */ -struct dispatch_t *pubsub_builder_finalize(pubsub_builder_t *); +struct dispatch_t *pubsub_builder_finalize(pubsub_builder_t *, + pubsub_items_t **items_out); + +/** + * Clear all pub_binding_t backpointers in <b>items</b>. + **/ +void pubsub_items_clear_bindings(pubsub_items_t *items);

-#ifdef PUBSUB_PRIVATE -struct pubsub_items_t; #define pubsub_items_free(cfg) \ FREE_AND_NULL(pubsub_items_t, pubsub_items_free_, (cfg)) -void pubsub_items_free_(struct pubsub_items_t *cfg); -#endif +void pubsub_items_free_(pubsub_items_t *cfg);

#endif diff --git a/src/lib/pubsub/pubsub_builder_st.h b/src/lib/pubsub/pubsub_builder_st.h index a1cc6e718..cedeb02b1 100644 --- a/src/lib/pubsub/pubsub_builder_st.h +++ b/src/lib/pubsub/pubsub_builder_st.h @@ -91,12 +91,12 @@ typedef struct pubsub_type_cfg_t { * The set of configuration requests for a dispatcher, as made by various * subsystems. **/ -typedef struct pubsub_items_t { +struct pubsub_items_t { /** List of pubsub_cfg_t. */ struct smartlist_t *items; /** List of pubsub_type_cfg_t. */ struct smartlist_t *type_items; -} pubsub_items_t; +};

/** * Type used to construct a dispatcher. We use this type to build up the @@ -111,7 +111,7 @@ struct pubsub_builder_t { int n_errors; /** In-progress configuration that we're constructing, as a list of the * requests that have been made. */ - pubsub_items_t *items; + struct pubsub_items_t *items; /** In-progress configuration that we're constructing, in a form that can * be converted to a dispatch_t. */ struct dispatch_cfg_t *cfg; diff --git a/src/test/test_pubsub_msg.c b/src/test/test_pubsub_msg.c index 5b771d45a..41a8a25ad 100644 --- a/src/test/test_pubsub_msg.c +++ b/src/test/test_pubsub_msg.c @@ -148,7 +148,7 @@ setup_dispatcher(const struct testcase_t *testcase) tor_free(sys); }

- return pubsub_builder_finalize(builder); + return pubsub_builder_finalize(builder, NULL); }

static int