lock: Avoid some undefined behaviour when freeing mutexes.

Fixes bug 31736; bugfix on 0.0.7.

2 files changed, 20 insertions, 2 deletions

diff --git a/src/lib/thread/compat_threads.c b/src/lib/thread/compat_threads.c
index 94ab021c52..16cece6125 100644
--- a/src/lib/thread/compat_threads.c
+++ b/src/lib/thread/compat_threads.c
@@ -65,7 +65,15 @@ atomic_counter_init(atomic_counter_t *counter)
   memset(counter, 0, sizeof(*counter));
   tor_mutex_init_nonrecursive(&counter->mutex);
 }
-/** Clean up all resources held by an atomic counter. */
+/** Clean up all resources held by an atomic counter.
+ *
+ * Destroying a locked mutex is undefined behaviour. Global mutexes may be
+ * locked when they are passed to this function, because multiple threads can
+ * still access them. So we can either:
+ *  - destroy on shutdown, and re-initialise when tor re-initialises, or
+ *  - skip destroying and re-initialisation, using a sentinel variable.
+ * See #31735 for details.
+ */
 void
 atomic_counter_destroy(atomic_counter_t *counter)
 {
diff --git a/src/lib/thread/threads.h b/src/lib/thread/threads.h
index ecf60641b5..de3da6a585 100644
--- a/src/lib/thread/threads.h
+++ b/src/lib/thread/threads.h
@@ -131,7 +131,17 @@ atomic_counter_init(atomic_counter_t *counter)
 {
   atomic_init(&counter->val, 0);
 }
-/** Clean up all resources held by an atomic counter. */
+/** Clean up all resources held by an atomic counter.
+ *
+ * This usage note applies to the compat_threads implementation of
+ * atomic_counter_destroy():
+ * Destroying a locked mutex is undefined behaviour. Global mutexes may be
+ * locked when they are passed to this function, because multiple threads can
+ * still access them. So we can either:
+ *  - destroy on shutdown, and re-initialise when tor re-initialises, or
+ *  - skip destroying and re-initialisation, using a sentinel variable.
+ * See #31735 for details.
+ */
 static inline void
 atomic_counter_destroy(atomic_counter_t *counter)
 {