Handle closing circuits correctly with circuitmux_t

1 files changed, 57 insertions, 45 deletions

diff --git a/src/or/circuitmux.c b/src/or/circuitmux.c
index 2bf511fe44..48336d8311 100644
--- a/src/or/circuitmux.c
+++ b/src/or/circuitmux.c
@@ -130,6 +130,8 @@ struct chanid_circid_muxinfo_t {
  * Internal-use #defines
  */
 
+#define CMUX_PARANOIA
+
 #ifdef CMUX_PARANOIA
 #define circuitmux_assert_okay_paranoid(cmux) \
   circuitmux_assert_okay(cmux)
@@ -1476,53 +1478,63 @@ circuitmux_assert_okay_pass_one(circuitmux_t *cmux)
     chan = channel_find_by_global_id(chan_id);
     tor_assert(chan);
     circ = circuit_get_by_circid_channel(circ_id, chan);
-    tor_assert(circ);
-
-    /* Clear the circ_is_active bit to start */
-    circ_is_active = 0;
-
-    /* Assert that we know which direction this is going */
-    tor_assert((*i)->muxinfo.direction == CELL_DIRECTION_OUT ||
-               (*i)->muxinfo.direction == CELL_DIRECTION_IN);
-
-    if ((*i)->muxinfo.direction == CELL_DIRECTION_OUT) {
-      /* We should be n_mux on this circuit */
-      tor_assert(cmux == circ->n_mux);
-      tor_assert(chan == circ->n_chan);
-      /* Get next and prev for next test */
-      next_p = &(circ->next_active_on_n_chan);
-      prev_p = &(circ->prev_active_on_n_chan);
-    } else {
-      /* This should be an or_circuit_t and we should be p_mux */
-      or_circ = TO_OR_CIRCUIT(circ);
-      tor_assert(cmux == or_circ->p_mux);
-      tor_assert(chan == or_circ->p_chan);
-      /* Get next and prev for next test */
-      next_p = &(or_circ->next_active_on_p_chan);
-      prev_p = &(or_circ->prev_active_on_p_chan);
-    }
+    if (circ) {
+      /* Clear the circ_is_active bit to start */
+      circ_is_active = 0;
+
+      /* Assert that we know which direction this is going */
+      tor_assert((*i)->muxinfo.direction == CELL_DIRECTION_OUT ||
+                 (*i)->muxinfo.direction == CELL_DIRECTION_IN);
+
+      if ((*i)->muxinfo.direction == CELL_DIRECTION_OUT) {
+        /* We should be n_mux on this circuit */
+        tor_assert(cmux == circ->n_mux);
+        tor_assert(chan == circ->n_chan);
+        /* Get next and prev for next test */
+        next_p = &(circ->next_active_on_n_chan);
+        prev_p = &(circ->prev_active_on_n_chan);
+      } else {
+        /* This should be an or_circuit_t and we should be p_mux */
+        or_circ = TO_OR_CIRCUIT(circ);
+        tor_assert(cmux == or_circ->p_mux);
+        tor_assert(chan == or_circ->p_chan);
+        /* Get next and prev for next test */
+        next_p = &(or_circ->next_active_on_p_chan);
+        prev_p = &(or_circ->prev_active_on_p_chan);
+      }
 
-    /*
-     * Should this circuit be active?  I.e., does the mux know about > 0
-     * cells on it?
-     */
-    circ_is_active = ((*i)->muxinfo.cell_count > 0);
-
-    /* It should be in the linked list iff it's active */
-    if (circ_is_active) {
-      /* Either we have a next link or we are the tail */
-      tor_assert(*next_p || (circ == cmux->active_circuits_tail));
-      /* Either we have a prev link or we are the head */
-      tor_assert(*prev_p || (circ == cmux->active_circuits_head));
-      /* Increment the active circuits counter */
-      ++n_active_circuits;
+      /*
+       * Should this circuit be active?  I.e., does the mux know about > 0
+       * cells on it?
+       */
+      circ_is_active = ((*i)->muxinfo.cell_count > 0);
+
+      /* It should be in the linked list iff it's active */
+      if (circ_is_active) {
+        /* Either we have a next link or we are the tail */
+        tor_assert(*next_p || (circ == cmux->active_circuits_tail));
+        /* Either we have a prev link or we are the head */
+        tor_assert(*prev_p || (circ == cmux->active_circuits_head));
+        /* Increment the active circuits counter */
+        ++n_active_circuits;
+      } else {
+        /* Shouldn't be in list, so no next or prev link */
+        tor_assert(!(*next_p));
+        tor_assert(!(*prev_p));
+        /* And can't be head or tail */
+        tor_assert(circ != cmux->active_circuits_head);
+        tor_assert(circ != cmux->active_circuits_tail);
+      }
     } else {
-      /* Shouldn't be in list, so no next or prev link */
-      tor_assert(!(*next_p));
-      tor_assert(!(*prev_p));
-      /* And can't be head or tail */
-      tor_assert(circ != cmux->active_circuits_head);
-      tor_assert(circ != cmux->active_circuits_tail);
+      /*
+       * circuit_get_by_circid_channel() doesn't want us to have a circuit
+       * that's marked for close, but we can test for that case with
+       * circuit_id_in_use_on_channel().  Assert if it really, really isn't
+       * there.
+       */
+      tor_assert(circuit_id_in_use_on_channel(circ_id, chan));
+      /* It definitely isn't active */
+      circ_is_active = 0;
     }
 
     /* Increment the circuits counter */