[tor-commits] [tor] branch main updated: cmux: Remove a log bug that is actually an acceptable race

[gitolite role]

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 at 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 at 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);

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.