From ed74c5215862e1464e4f72b9c5995613ea00e9c8 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 26 Jul 2022 11:45:43 -0400 Subject: [PATCH] cmux: Remove a log bug that is actually an acceptable race Closes #40647 Signed-off-by: David Goulet --- changes/ticket40647 | 4 ++++ src/core/or/relay.c | 21 +++++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 changes/ticket40647 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);