conflux: Flag set as in full teardown in the free path

We suspect a shutdown race of some sort for which the full teardown is not
noticed during the close but should be during the free.

For that, we flag the conflux set as in full teardown (if so) in the free path
in case the close path didn't caught it.

Fixes #40870

Signed-off-by: David Goulet <dgoulet@torproject.org>

changes/ticket40870

@@ -0,0 +1,4 @@
+  o Minor bugfixes (conflux, client):
+    - Avoid a non fatal assert caused by data coming in on a conflux set that is
+      being freed during shutdown. Fixes bug 40870; bugfix on 0.4.8.1-alpha.
+

src/app/main/shutdown.c

@@ -120,6 +120,7 @@ tor_free_all(int postfork)
   dirserv_free_all();
   rep_hist_free_all();
   bwhist_free_all();
+  conflux_notify_shutdown();
   circuit_free_all();
   conflux_pool_free_all();
   circpad_machines_free();

src/core/or/conflux_pool.c

@@ -1607,7 +1607,22 @@ linked_circuit_free(circuit_t *circ, bool is_client)
 
   /* Circuit can be freed without being closed and so we try to delete this leg
    * so we can learn if this circuit is the last leg or not. */
-  cfx_del_leg(circ->conflux, circ);
+  if (cfx_del_leg(circ->conflux, circ)) {
+    /* Check for instances of bug #40870, which we suspect happen
+     * during exit. If any happen outside of exit, BUG and warn. */
+    if (!circ->conflux->in_full_teardown) {
+      /* We should bug and warn if we're not in a shutdown process; that
+       * means we got here somehow without a close. */
+      if (BUG(!shutting_down)) {
+        log_warn(LD_BUG,
+                 "Conflux circuit %p being freed without being marked for "
+                 "full teardown via close, with shutdown state %d. "
+                 "Please report this.", circ, shutting_down);
+        conflux_log_set(LOG_WARN, circ->conflux, is_client);
+      }
+      circ->conflux->in_full_teardown = true;
+    }
+  }
 
   if (CONFLUX_NUM_LEGS(circ->conflux) > 0) {
     /* The last leg will free the streams but until then, we nullify to avoid
@@ -2126,14 +2141,36 @@ conflux_log_set(int loglevel, const conflux_t *cfx, bool is_client)
   }
 }
 
+/**
+ * Conflux needs a notification when tor_shutdown() begins, so that
+ * when circuits are freed, new legs are not launched.
+ *
+ * This needs a separate notification from conflux_pool_free_all(),
+ * because circuits must be freed before that function.
+ */
+void
+conflux_notify_shutdown(void)
+{
+  shutting_down = true;
+}
+
+#ifdef TOR_UNIT_TESTS
+/**
+ * For unit tests: Clear the shutting down state so we resume building legs.
+ */
+void
+conflux_clear_shutdown(void)
+{
+  shutting_down = false;
+}
+#endif
+
 /** Free and clean up the conflux pool subsystem. This is called by the subsys
  * manager AFTER all circuits have been freed which implies that all objects in
  * the pools aren't referenced anymore. */
 void
 conflux_pool_free_all(void)
 {
-  shutting_down = true;
-
   digest256map_free(client_linked_pool, free_conflux_void_);
   digest256map_free(server_linked_pool, free_conflux_void_);
   digest256map_free(client_unlinked_pool, free_unlinked_void_);

src/core/or/conflux_pool.h

@@ -12,6 +12,7 @@
 #include "core/or/or.h"
 
 void conflux_pool_init(void);
+void conflux_notify_shutdown(void);
 void conflux_pool_free_all(void);
 
 origin_circuit_t *conflux_get_circ_for_conn(const entry_connection_t *conn,
@@ -41,6 +42,7 @@ void conflux_log_set(int loglevel, const conflux_t *cfx, bool is_client);
 
 #ifdef TOR_UNIT_TESTS
 bool launch_new_set(int num_legs);
+void conflux_clear_shutdown(void);
 digest256map_t *get_linked_pool(bool is_client);
 digest256map_t *get_unlinked_pool(bool is_client);
 extern uint8_t DEFAULT_CLIENT_UX;

src/test/test_conflux_pool.c

@@ -396,6 +396,7 @@ test_setup(void)
 static void
 test_clear_circs(void)
 {
+  conflux_notify_shutdown();
   SMARTLIST_FOREACH(circ_pairs, circ_pair_t *, circ_pair, {
     tor_free(circ_pair);
   });
@@ -430,6 +431,9 @@ test_clear_circs(void)
   tor_assert(smartlist_len(mock_cell_delivery) == 0);
 
   (void)free_fake_origin_circuit;
+
+  /* Clear shutdown flag so we can resume testing again. */
+  conflux_clear_shutdown();
 }
 
 static void