Browse Source

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.
Nick Mathewson 6 years ago
parent
commit
a7681525ab

+ 25 - 5
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.
@@ -254,13 +254,29 @@ dispatch_fill_pub_binding_backptrs(pubsub_builder_t *builder,
   } SMARTLIST_FOREACH_END(cfg);
 }
 
+/**
+ * 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);

+ 15 - 5
src/lib/pubsub/pubsub_build.h

@@ -32,6 +32,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

+ 3 - 3
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;

+ 1 - 1
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