[tor-commits] [tor/maint-0.2.3] Disable warning for marked-but-reading in main.c

[nickm at torproject.org]

commit 0600e8cab17d2128d663b31d93b93ddc1ab95ee2
Author: Nick Mathewson <nickm at torproject.org>
Date:   Tue Jun 19 11:48:03 2012 -0400

    Disable warning for marked-but-reading in main.c
    
    It turns out this can happen.  Even though there is no reason for
    connections to be marked but reading, we leave them reading anyway,
    so warning here is unwarranted.  Let's turn that back on once we do
    something sensible and disable reading when we mark.  Bugfix for
    6203 on Tor 0.2.3.17-beta.
    
    Thanks to cypherpunks for pointing out the general stupidity of the
    original code here.
---
 changes/bug6203 |    5 +++++
 src/or/main.c   |   13 ++++++++++++-
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/changes/bug6203 b/changes/bug6203
new file mode 100644
index 0000000..bf96753
--- /dev/null
+++ b/changes/bug6203
@@ -0,0 +1,5 @@
+  o Minor bugfixes:
+    - Disable a spurious warning about reading on a marked and flushing
+      connection. We shouldn't be doing that, but apparently we
+      sometimes do. Fix for bug 6203; bugfix on 0.2.3.17-beta.
+
diff --git a/src/or/main.c b/src/or/main.c
index f8740d7..1f3117b 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -848,7 +848,10 @@ conn_close_if_marked(int i)
                            "Holding conn (fd %d) open for more flushing.",
                            (int)conn->s));
         conn->timestamp_lastwritten = now; /* reset so we can flush more */
-      } else if (sz == 0) { /* retval is also 0 */
+      } else if (sz == 0) {
+        /* Also, retval==0.  If we get here, we didn't want to write anything
+         * (because of rate-limiting) and we didn't. */
+
         /* Connection must flush before closing, but it's being rate-limited.
          * Let's remove from Libevent, and mark it as blocked on bandwidth
          * so it will be re-added on next token bucket refill. Prevents
@@ -860,6 +863,13 @@ conn_close_if_marked(int i)
           connection_stop_writing(conn);
         }
         if (connection_is_reading(conn)) {
+          /* XXXX024 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 */
+#if 0
 #define MARKED_READING_RATE 180
           static ratelim_t marked_read_lim = RATELIM_INIT(MARKED_READING_RATE);
           char *m;
@@ -870,6 +880,7 @@ conn_close_if_marked(int i)
                      conn_state_to_string(conn->type, conn->state), m);
             tor_free(m);
           }
+#endif
           conn->read_blocked_on_bw = 1;
           connection_stop_reading(conn);
         }