[tor-commits] [tor/main] edge: Remove wrong bug warn when processing pending streams

[dgoulet at torproject.org]

commit 4046b9f3ee996258e2335c64e7b914df7185ffc9
Author: David Goulet <dgoulet at torproject.org>
Date:   Mon Oct 4 14:11:18 2021 -0400

    edge: Remove wrong bug warn when processing pending streams
    
    The connection_ap_attach_pending() function processes all pending
    streams in the pending_entry_connections list. It first copy the pointer
    and then allocates a brand new empty list.
    
    It then iterates over that copy pointer to try to attach entry
    connections onto any fitting circuits using
    connection_ap_handshake_attach_circuit().
    
    That very function, for onion service, can lead to flagging _all_
    streams of the same onion service to be put in state RENDDESC_WAIT from
    CIRCUIT_WAIT. By doing so, it also tries to remove them from the
    pending_entry_connections but at that point it is already empty.
    
    Problem is that the we are iterating over the previous
    pending_entry_connections which contains the streams that have just
    changed state and are no longer in CIRCUIT_WAIT.
    
    This lead to this bug warning occuring a lot on busy services:
    
      May 01 08:55:43.000 [warn] connection_ap_attach_pending(): Bug:
      0x55d8764ae550 is no longer in circuit_wait. Its current state is
      waiting for rendezvous desc. Why is it on pending_entry_connections?
      (on Tor 0.4.4.0-alpha-dev )
    
    This fix is minimal and basically allow a state to be not CIRCUIT_WAIT
    and move on to the next one without logging a warning. Because the
    pending_entry_connections is emptied before processing, there is no
    chance for a streams to be stuck there forever thus it is OK to ignore
    streams not in the right state.
    
    Fixes #34083
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 changes/ticket34083           | 5 +++++
 src/core/or/connection_edge.c | 9 ++++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/changes/ticket34083 b/changes/ticket34083
new file mode 100644
index 0000000000..417d01c5a5
--- /dev/null
+++ b/changes/ticket34083
@@ -0,0 +1,5 @@
+  o Minor bugfixes (onion service):
+    - Fix a warning BUG that would occur often on heavily loaded onion service
+      leading to filling the logs with useless warnings. Fixes bug 34083; bugfix
+      on 0.3.2.1-alpha.
+
diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c
index d4d9d2f759..4228cd6aba 100644
--- a/src/core/or/connection_edge.c
+++ b/src/core/or/connection_edge.c
@@ -1347,11 +1347,10 @@ connection_ap_attach_pending(int retry)
       continue;
     }
     if (conn->state != AP_CONN_STATE_CIRCUIT_WAIT) {
-      log_warn(LD_BUG, "%p is no longer in circuit_wait. Its current state "
-               "is %s. Why is it on pending_entry_connections?",
-               entry_conn,
-               conn_state_to_string(conn->type, conn->state));
-      UNMARK();
+      /* The connection_ap_handshake_attach_circuit() call, for onion service,
+       * can lead to more than one connections in the "pending" list to change
+       * state and so it is OK to get here. Ignore it because this connection
+       * won't be in pending_entry_connections list after this point. */
       continue;
     }