[tor-commits] [tor/master] conn: Close the read side of a closing connection when write limit is reached

[nickm@xxxxxxxxxxxxxx]

commit c99f220f7857e40bd9c09bd0c240b3b9aea4d6f8
Author: David Goulet <dgoulet@xxxxxxxxxxxxxx>
Date:   Tue Nov 13 10:22:10 2018 -0500

    conn: Close the read side of a closing connection when write limit is reached
    
    In conn_close_if_marked(), we can decide to keep a connection open that still
    has data to flush on the wire if it is being rate limited on the write side.
    
    However, in this process, we were also looking at the read() side which can
    still have token in its bucket and thus not stop the reading. This lead to a
    BUG() introduced in 0.3.4.1-alpha that was expecting the read side to be
    closed due to the rate limit but which only applies on the write side.
    
    This commit removes any bandwidth check on the read side and simply stop the
    read side on the connection regardless of the bucket state. If we keep the
    connection open to flush it out before close, we should not read anything.
    
    Fixes #27750
    
    Signed-off-by: David Goulet <dgoulet@xxxxxxxxxxxxxx>
---
 changes/bug27750 |  6 ++++++
 src/or/main.c    | 17 ++++++-----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/changes/bug27750 b/changes/bug27750
new file mode 100644
index 000000000..c234788b1
--- /dev/null
+++ b/changes/bug27750
@@ -0,0 +1,6 @@
+  o Minor bugfixes (connection, relay):
+    - Avoid a wrong BUG() stacktrace in case a closing connection is being held
+      open because the write side is rate limited but not the read side. Now,
+      the connection read side is simply shutdown instead of kept open until tor
+      is able to flush the connection and then fully close it. Fixes bug 27750;
+      bugfix on 0.3.4.1-alpha.
diff --git a/src/or/main.c b/src/or/main.c
index bc01e07c3..ccfc14467 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -1036,23 +1036,18 @@ conn_close_if_marked(int i)
          * busy Libevent loops where we keep ending up here and returning
          * 0 until we are no longer blocked on bandwidth.
          */
-        connection_consider_empty_read_buckets(conn);
         connection_consider_empty_write_buckets(conn);
-
         /* Make sure that consider_empty_buckets really disabled the
          * connection: */
         if (BUG(connection_is_writing(conn))) {
           connection_write_bw_exhausted(conn, true);
         }
-        if (BUG(connection_is_reading(conn))) {
-          /* XXXX+ We should make this code unreachable; if a connection is
-           * marked for close and flushing, there is no point in reading to it
-           * at all. Further, checking at this point is a bit of a hack: it
-           * would make much more sense to react in
-           * connection_handle_read_impl, or to just stop reading in
-           * mark_and_flush */
-          connection_read_bw_exhausted(conn, true/* kludge. */);
-        }
+
+        /* The connection is being held due to write rate limit and thus will
+         * flush its data later. We need to stop reading because this
+         * connection is about to be closed once flushed. It should not
+         * process anything more coming in at this stage. */
+        connection_stop_reading(conn);
       }
       return 0;
     }



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits