When clearing cells from a circuit for OOM reasons, tell cmux we did so.

Not telling the cmux would sometimes cause an assertion failure in
relay.c when we tried to get an active circuit and found an "active"
circuit with no cells.

Additionally, replace that assert with a test and a log message.

Fix for bug 20203. This is actually probably a bugfix on
0.2.8.1-alpha, specifically my code in 8b4e5b7ee902fb7fa0776 where I
made circuit_mark_for_close_() do less in order to simplify our call
graph. Thanks to "cypherpunks" for help diagnosing.

diff --git a/changes/bug20203 b/changes/bug20203
new file mode 100644 (file)
index 0000000..b4809ff
--- /dev/null
+++ b/changes/bug20203
@@ -0,0 +1,6 @@
+  o Major bugfixes (relay, OOM handler)
+    - Fix a timing-dependent assertion failure that could occur when we
+      tried to flush from a circuit after having freed its cells because
+      of an out-of-memory condition. Fixes bug 20203; bugfix on
+      0.2.8.1-alpha. Thanks to "cypherpunks" for help diagnosing this
+      one.

diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 716024df6aab51e98bb9724d5da1d62a2fb53d9f..5fb8be54b6f5fe6be56879200a7e32ed4c15a2ba 100644 (file)
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -1836,8 +1836,14 @@ marked_circuit_free_cells(circuit_t *circ)
     return;
   }
   cell_queue_clear(&circ->n_chan_cells);
-  if (! CIRCUIT_IS_ORIGIN(circ))
-    cell_queue_clear(& TO_OR_CIRCUIT(circ)->p_chan_cells);
+  if (circ->n_mux)
+    circuitmux_clear_num_cells(circ->n_mux, circ);
+  if (! CIRCUIT_IS_ORIGIN(circ)) {
+    or_circuit_t *orcirc = TO_OR_CIRCUIT(circ);
+    cell_queue_clear(&orcirc->p_chan_cells);
+    if (orcirc->p_mux)
+      circuitmux_clear_num_cells(orcirc->p_mux, circ);
+  }
 }
 
 static size_t

diff --git a/src/or/relay.c b/src/or/relay.c
index eddad6a0cb344f786012fbca13fd1596c64fae31..33191e1e89976d5a1ea9447efe8d7fc336609d3c 100644 (file)
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -2619,6 +2619,15 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max))
     }
 
     /* Circuitmux told us this was active, so it should have cells */
+    if (/*BUG(*/ queue->n == 0 /*)*/) {
+      log_warn(LD_BUG, "Found a supposedly active circuit with no cells "
+               "to send. Trying to recover.");
+      circuitmux_set_num_cells(cmux, circ, 0);
+      if (! circ->marked_for_close)
+        circuit_mark_for_close(circ, END_CIRC_REASON_INTERNAL);
+      continue;
+    }
+
     tor_assert(queue->n > 0);
 
     /*