Close a non-open OR connection *only* after KeepalivePeriod.

When we introduced the code to close non-open OR connections after
KeepalivePeriod had passed, we replaced some code that said
    if (!connection_is_open(conn)) {
     /* let it keep handshaking forever */
    } else if (do other tests here) {
      ...
with new code that said
    if (!connection_is_open(conn) && past_keepalive) {
     /* let it keep handshaking forever */
    } else if (do other tests here) {
      ...

This was a mistake, since it made all the other tests start applying
to non-open connections, thus causing bug 1840, where non-open
connections get closed way early.

Fixes bug 1840.  Bugfix on 0.2.1.26 (commit 67b38d50).

diff --git a/changes/bug1840 b/changes/bug1840
new file mode 100644 (file)
index 0000000..0ef2b98
--- /dev/null
+++ b/changes/bug1840
@@ -0,0 +1,7 @@
+  o Minor bugfixes:
+    - Allow handshaking OR connections to take a full KeepalivePeriod
+      seconds to handshake.  Previously, we would close them after
+      IDLE_OR_CONN_TIMEOUT seconds, as if they were open.  This is a
+      bugfix on 0.2.1.26.  Thanks to mingw-san for analysis help.  Fixes
+      bug 1840.
+

diff --git a/src/or/main.c b/src/or/main.c
index 9052f7c570b7f0dc6e7b98f47ab7a3145f8a4637..174c874f6e6aab5d07e77a2710dcda07419ebe9c 100644 (file)
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -735,12 +735,15 @@ run_connection_housekeeping(int i, time_t now)
                                    "Tor gave up on the connection");
     connection_mark_for_close(conn);
     conn->hold_open_until_flushed = 1;
-  } else if (past_keepalive && !connection_state_is_open(conn)) {
-    /* We never managed to actually get this connection open and happy. */
-    log_info(LD_OR,"Expiring non-open OR connection to fd %d (%s:%d).",
-             conn->s,conn->address, conn->port);
-    connection_mark_for_close(conn);
-    conn->hold_open_until_flushed = 1;
+  } else if (!connection_state_is_open(conn)) {
+    if (past_keepalive) {
+      /* We never managed to actually get this connection open and happy. */
+      log_info(LD_OR,"Expiring non-open OR connection to fd %d (%s:%d).",
+               conn->s,conn->address, conn->port);
+      connection_mark_for_close(conn);
+      conn->hold_open_until_flushed = 1; /* XXXX why? I think we can remove
+                                          * this. -NM */
+    }
   } else if (we_are_hibernating() && !or_conn->n_circuits &&
              !buf_datalen(conn->outbuf)) {
     /* We're hibernating, there's no circuits, and nothing to flush.*/