aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Goulet <dgoulet@torproject.org>2022-07-26 11:45:43 -0400
committerDavid Goulet <dgoulet@torproject.org>2022-07-26 16:14:09 -0400
commited74c5215862e1464e4f72b9c5995613ea00e9c8 (patch)
treee6d2d37682e18c4fa10f6536840931dd6c8be252
parentda52d7206a4a8e4fa8b5e80b5ed73de50fbe8692 (diff)
downloadtor-ed74c5215862e1464e4f72b9c5995613ea00e9c8.tar.gz
tor-ed74c5215862e1464e4f72b9c5995613ea00e9c8.zip
cmux: Remove a log bug that is actually an acceptable race
Closes #40647 Signed-off-by: David Goulet <dgoulet@torproject.org>
-rw-r--r--changes/ticket406474
-rw-r--r--src/core/or/relay.c21
2 files changed, 21 insertions, 4 deletions
diff --git a/changes/ticket40647 b/changes/ticket40647
new file mode 100644
index 0000000000..ae20aae3f3
--- /dev/null
+++ b/changes/ticket40647
@@ -0,0 +1,4 @@
+ o Minor bugfixes (relay):
+ - Remove a "BUG" warning for an acceptable race between a circuit close
+ and considering that circuit active. Fixes bug 40647; bugfix on
+ 0.3.5.1-alpha.
diff --git a/src/core/or/relay.c b/src/core/or/relay.c
index 68fddd1ae7..332878a8ea 100644
--- a/src/core/or/relay.c
+++ b/src/core/or/relay.c
@@ -3038,10 +3038,23 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max))
streams_blocked = circ->streams_blocked_on_p_chan;
}
- /* 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 told us this was active, so it should have cells.
+ *
+ * Note: In terms of logic and coherence, this should never happen but the
+ * cmux dragon is powerful. Reason is that when the OOM is triggered, when
+ * cleaning up circuits, we mark them for close and then clear their cell
+ * queues. And so, we can have a circuit considered active by the cmux
+ * dragon but without cells. The cmux subsystem is only notified of this
+ * when the circuit is freed which leaves a tiny window between close and
+ * free to end up here.
+ *
+ * We are accepting this as an "ok" race else the changes are likely non
+ * trivial to make the mark for close to set the num cells to 0 and change
+ * the free functions to detach the circuit conditionnaly without creating
+ * a chain effect of madness.
+ *
+ * The lesson here is arti will prevail and leave the cmux dragon alone. */
+ if (queue->n == 0) {
circuitmux_set_num_cells(cmux, circ, 0);
if (! circ->marked_for_close)
circuit_mark_for_close(circ, END_CIRC_REASON_INTERNAL);