This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
The following commit(s) were added to refs/heads/main by this push: new ed74c52158 cmux: Remove a log bug that is actually an acceptable race ed74c52158 is described below
commit ed74c5215862e1464e4f72b9c5995613ea00e9c8 Author: David Goulet dgoulet@torproject.org AuthorDate: Tue Jul 26 11:45:43 2022 -0400
cmux: Remove a log bug that is actually an acceptable race
Closes #40647
Signed-off-by: David Goulet dgoulet@torproject.org --- changes/ticket40647 | 4 ++++ src/core/or/relay.c | 21 +++++++++++++++++---- 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);