r18031@catbus: nickm | 2008-02-11 13:54:58 -0500

 Have assert_connection_ok() allow marked-for-close dir conns with stuff to flush but no way to flush it.  Adjust conn_dirserv_unlink_from_bridge() to mark edge and dir conns not already marked, since once linked conns are unlinked they are no longer viable.  Likely fix for bug 406, which was crashing 0.1.2.x servers periodically.


svn:r13469

3 files changed, 14 insertions, 3 deletions

diff --git a/ChangeLog b/ChangeLog
index ede44c1464..99f1b2acea 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,10 @@ Changes in versino 0.1.2.20 - 2008-??-??
     - Patch from "Andrew S. Lists" to catch when we contact a directory
       mirror at IP address X and he says we look like we're coming from
       IP address X. Bugfix on 0.1.2.x.
+    - Allow a closing-down linked directory connection to have its
+      blocked_on_or_conn field set.  This prevents a rare assertion error
+      that could occur when an OR connection carrying tunneled directory
+      requests closed before the requests were complete.  Fixes bug 406.
 
   o Minor bugfixes:
     - Stop recommending that every server operator send mail to tor-ops.
diff --git a/src/or/connection.c b/src/or/connection.c
index 04fe25dce2..88de54d62e 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2403,7 +2403,8 @@ assert_connection_ok(connection_t *conn, time_t now)
   if (conn->outbuf_flushlen > 0) {
     tor_assert(connection_is_writing(conn) || conn->wants_to_write ||
                (conn->type == CONN_TYPE_DIR &&
-                TO_DIR_CONN(conn)->is_blocked_on_or_conn));
+                (conn->marked_for_close ||
+                 TO_DIR_CONN(conn)->is_blocked_on_or_conn)));
   }
 
   if (conn->hold_open_until_flushed)
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index a2ff421ebf..f61b9feb03 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -2051,12 +2051,18 @@ connection_dirserv_unlink_from_bridge(dir_connection_t *dir_conn)
   or_conn = connection_dirserv_get_target_or_conn(dir_conn);
   if (or_conn) {
     /* XXXX Really, this is only necessary if dir_conn->is_blocked_on_or_conn.
-     * But for now, let's leave it in, so the assert can catch  */
+     * But for now, let's leave it in, so the assert can catch problems.  */
     connection_dirserv_remove_from_blocked_list(or_conn, dir_conn);
   }
   dir_conn->is_blocked_on_or_conn = 0; /* Probably redundant. */
-  edge_conn->bridge_for_conn = NULL;
   dir_conn->bridge_conn = NULL;
+  if (edge_conn) {
+    edge_conn->bridge_for_conn = NULL;
+    if (!edge_conn->_base.marked_for_close)
+      connection_mark_for_close(TO_CONN(edge_conn));
+  }
+  if (!dir_conn->_base.marked_for_close)
+    connection_mark_for_close(TO_CONN(dir_conn));
 }
 
 /** Stop writing on a bridged dir_conn, and remember that it's blocked because