Fix: call_rcu: teardown default call_rcu worker on application exit

Teardown the default call_rcu worker thread if there are no queued
callbacks on process exit. This prevents leaking memory.

Here is how an application can ensure graceful teardown of this
worker thread:

- An application queuing call_rcu callbacks should invoke
  rcu_barrier() before it exits.
- When chaining call_rcu callbacks, the number of calls to
  rcu_barrier() on application exit must match at least the maximum
  number of chained callbacks.
- If an application chains callbacks endlessly, it would have to be
  modified to stop chaining callbacks when it detects an application
  exit (e.g. with a flag), and wait for quiescence with rcu_barrier()
  after setting that flag.
- The statements above apply to a library which queues call_rcu
  callbacks, only it needs to invoke rcu_barrier in its library
  destructor.

Fixes: #1317
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: I40556bc872d3df58a22fb88a0dbb528ce5c9b4af

diff --git a/src/urcu-bp.c b/src/urcu-bp.c
index 671becb0e14644c6eeb37270ab154f1b95877549..35ac9dbed12b01e19166e93e5f176a9c2dc490da 100644 (file)
--- a/src/urcu-bp.c
+++ b/src/urcu-bp.c
@@ -121,6 +121,7 @@ static
 void __attribute__((constructor)) _urcu_bp_init(void);
 static
 void __attribute__((destructor)) urcu_bp_exit(void);
+static void urcu_call_rcu_exit(void);
 
 #ifndef CONFIG_RCU_FORCE_SYS_MEMBARRIER
 int urcu_bp_has_sys_membarrier;
@@ -662,6 +663,8 @@ void _urcu_bp_init(void)
 static
 void urcu_bp_exit(void)
 {
+       urcu_call_rcu_exit();
+
        mutex_lock(&init_lock);
        if (!--urcu_bp_refcount) {
                struct registry_chunk *chunk, *tmp;

diff --git a/src/urcu-call-rcu-impl.h b/src/urcu-call-rcu-impl.h
index e34faee68c3c642e96cba3bfb06f242ab55228a8..15c1ae60a0eb035e8479a1c1e805e28366f9ec6b 100644 (file)
--- a/src/urcu-call-rcu-impl.h
+++ b/src/urcu-call-rcu-impl.h
@@ -452,8 +452,8 @@ static void call_rcu_data_init(struct call_rcu_data **crdpp,
        cds_list_add(&crdp->list, &call_rcu_data_list);
        crdp->cpu_affinity = cpu_affinity;
        crdp->gp_count = 0;
-       cmm_smp_mb();  /* Structure initialized before pointer is planted. */
-       *crdpp = crdp;
+       rcu_set_pointer(crdpp, crdp);
+
        ret = pthread_create(&crdp->tid, NULL, call_rcu_thread, crdp);
        if (ret)
                urcu_die(ret);
@@ -576,22 +576,27 @@ int alias_set_cpu_call_rcu_data();
 
 /*
  * Return a pointer to the default call_rcu_data structure, creating
- * one if need be.  Because we never free call_rcu_data structures,
- * we don't need to be in an RCU read-side critical section.
+ * one if need be.
+ *
+ * The call to this function with intent to use the returned
+ * call_rcu_data should be protected by RCU read-side lock.
  */
 
 struct call_rcu_data *get_default_call_rcu_data(void)
 {
-       if (default_call_rcu_data != NULL)
-               return rcu_dereference(default_call_rcu_data);
+       struct call_rcu_data *crdp;
+
+       crdp = rcu_dereference(default_call_rcu_data);
+       if (crdp != NULL)
+               return crdp;
+
        call_rcu_lock(&call_rcu_mutex);
-       if (default_call_rcu_data != NULL) {
-               call_rcu_unlock(&call_rcu_mutex);
-               return default_call_rcu_data;
-       }
-       call_rcu_data_init(&default_call_rcu_data, 0, -1);
+       if (default_call_rcu_data == NULL)
+               call_rcu_data_init(&default_call_rcu_data, 0, -1);
+       crdp = default_call_rcu_data;
        call_rcu_unlock(&call_rcu_mutex);
-       return default_call_rcu_data;
+
+       return crdp;
 }
 URCU_ATTR_ALIAS(urcu_stringify(get_default_call_rcu_data))
 struct call_rcu_data *alias_get_default_call_rcu_data();
@@ -1099,6 +1104,62 @@ void urcu_unregister_rculfhash_atfork(struct urcu_atfork *atfork __attribute__((
 {
        urcu_die(EPERM);
 }
+
 URCU_ATTR_ALIAS(urcu_stringify(urcu_unregister_rculfhash_atfork))
 __attribute__((noreturn))
 void alias_urcu_unregister_rculfhash_atfork();
+
+/*
+ * Teardown the default call_rcu worker thread if there are no queued
+ * callbacks on process exit. This prevents leaking memory.
+ *
+ * Here is how an application can ensure graceful teardown of this
+ * worker thread:
+ *
+ * - An application queuing call_rcu callbacks should invoke
+ *   rcu_barrier() before it exits.
+ * - When chaining call_rcu callbacks, the number of calls to
+ *   rcu_barrier() on application exit must match at least the maximum
+ *   number of chained callbacks.
+ * - If an application chains callbacks endlessly, it would have to be
+ *   modified to stop chaining callbacks when it detects an application
+ *   exit (e.g. with a flag), and wait for quiescence with rcu_barrier()
+ *   after setting that flag.
+ * - The statements above apply to a library which queues call_rcu
+ *   callbacks, only it needs to invoke rcu_barrier in its library
+ *   destructor.
+ *
+ * Note that this function does not presume it is being called when the
+ * application is single-threaded even though this is invoked from a
+ * destructor: this function synchronizes against concurrent calls to
+ * get_default_call_rcu_data().
+ */
+static void urcu_call_rcu_exit(void)
+{
+       struct call_rcu_data *crdp;
+       bool teardown = true;
+
+       if (default_call_rcu_data == NULL)
+               return;
+       call_rcu_lock(&call_rcu_mutex);
+       /*
+        * If the application leaves callbacks in the default call_rcu
+        * worker queue, keep the default worker in place.
+        */
+       crdp = default_call_rcu_data;
+       if (!crdp) {
+               teardown = false;
+               goto unlock;
+       }
+       if (!cds_wfcq_empty(&crdp->cbs_head, &crdp->cbs_tail)) {
+               teardown = false;
+               goto unlock;
+       }
+       rcu_set_pointer(&default_call_rcu_data, NULL);
+unlock:
+       call_rcu_unlock(&call_rcu_mutex);
+       if (teardown) {
+               synchronize_rcu();
+               call_rcu_data_free(crdp);
+       }
+}

diff --git a/src/urcu-qsbr.c b/src/urcu-qsbr.c
index 5572d39da65f1030510274b0618e301e893cfbcd..e5ac0539c47072154084bdb518b8a559076b1e55 100644 (file)
--- a/src/urcu-qsbr.c
+++ b/src/urcu-qsbr.c
@@ -53,6 +53,7 @@
 #define _LGPL_SOURCE
 
 void __attribute__((destructor)) urcu_qsbr_exit(void);
+static void urcu_call_rcu_exit(void);
 
 /*
  * rcu_gp_lock ensures mutual exclusion between threads calling
@@ -524,6 +525,7 @@ void urcu_qsbr_exit(void)
         * readers, and left running at exit.
         * assert(cds_list_empty(&registry));
         */
+       urcu_call_rcu_exit();
 }
 URCU_ATTR_ALIAS("urcu_qsbr_exit") void rcu_exit_qsbr();
 

diff --git a/src/urcu.c b/src/urcu.c
index a8c033a0bb7c960755428ec7a3caeff6b3f309e7..d81e84fde88e27de5c6ac28145c03cc5c6cf989d 100644 (file)
--- a/src/urcu.c
+++ b/src/urcu.c
@@ -113,9 +113,11 @@ void alias_rcu_init(void);
 static int init_done;
 
 void __attribute__((constructor)) rcu_init(void);
-void __attribute__((destructor)) rcu_exit(void);
 #endif
 
+void __attribute__((destructor)) rcu_exit(void);
+static void urcu_call_rcu_exit(void);
+
 /*
  * rcu_gp_lock ensures mutual exclusion between threads calling
  * synchronize_rcu().
@@ -671,22 +673,24 @@ void rcu_init(void)
 URCU_ATTR_ALIAS(urcu_stringify(rcu_init))
 void alias_rcu_init(void);
 
+/*
+ * Don't unregister the SIGRCU signal handler anymore, because
+ * call_rcu threads could still be using it shortly before the
+ * application exits.
+ * Assertion disabled because call_rcu threads are now rcu
+ * readers, and left running at exit.
+ * assert(cds_list_empty(&registry));
+ */
+
+#endif /* #ifdef RCU_SIGNAL */
+
 void rcu_exit(void)
 {
-       /*
-        * Don't unregister the SIGRCU signal handler anymore, because
-        * call_rcu threads could still be using it shortly before the
-        * application exits.
-        * Assertion disabled because call_rcu threads are now rcu
-        * readers, and left running at exit.
-        * assert(cds_list_empty(&registry));
-        */
+       urcu_call_rcu_exit();
 }
 URCU_ATTR_ALIAS(urcu_stringify(rcu_exit))
 void alias_rcu_exit(void);
 
-#endif /* #ifdef RCU_SIGNAL */
-
 DEFINE_RCU_FLAVOR(rcu_flavor);
 DEFINE_RCU_FLAVOR_ALIAS(rcu_flavor, alias_rcu_flavor);