[tor-commits] [tor/master] Add function to clear publish bindings.

[asn at torproject.org]

commit a7681525ab670c2b7a783f9e1285bf9a8e97d1ea
Author: Nick Mathewson <nickm at 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