r18862@catbus: nickm | 2008-03-16 23:33:11 -0400

 Part of fix for bug 617: allow connection_ap_handshake_attach_circuit() to mark connections, to avoid double-mark warnings.  Note that this is an incomplete refactoring.


svn:r14066

5 files changed, 25 insertions, 8 deletions

diff --git a/ChangeLog b/ChangeLog
index 1dc0f208c..34feaeabe 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -28,6 +28,7 @@ Changes in version 0.2.1.1-alpha - 2008-??-??
       0.2.0.x.
     - Detect mismatched page sizes when using --enable-openbsd-malloc.
       Bugfix on 0.2.0.x.
+    - Stop giving double-close warn when we reject an address for client DNS.
 
   o Minor features:
     - Allow separate log levels to be configured for different logging
@@ -38,6 +39,11 @@ Changes in version 0.2.1.1-alpha - 2008-??-??
       to avoid unused RAM in buffer chunks and memory pools.
     - Downgrade "sslv3 alert handshake failure" message to INFO.
 
+  o Code simplifications and refactoring:
+    - Refactor code using connection_ap_handshake_attach_circuit() to
+      allow that function to mark connections for close.  Part of a fix for
+      bug 617.  Bugfix on 0.2.0.1-alpha.
+
 
 Changes in version 0.2.0.21-rc - 2008-03-02
   o Major bugfixes:
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 5f4529285..d7fcf907a 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -1255,11 +1255,13 @@ connection_ap_handshake_attach_chosen_circuit(edge_connection_t *conn,
 
 /** Try to find a safe live circuit for CONN_TYPE_AP connection conn. If
  * we don't find one: if conn cannot be handled by any known nodes,
- * warn and return -1 (conn needs to die);
+ * warn and return -1 (conn needs to die, and is maybe already marked);
  * else launch new circuit (if necessary) and return 0.
  * Otherwise, associate conn with a safe live circuit, do the
  * right next step, and return 1.
  */
+/* XXXX021 this function should mark for close whenever it returns -1;
+ * its callers shouldn't have to worry about that. */
 int
 connection_ap_handshake_attach_circuit(edge_connection_t *conn)
 {
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 2cb8e5e80..549c20c78 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -431,7 +431,8 @@ connection_ap_expire_beginning(void)
     /* move it back into 'pending' state, and try to attach. */
     if (connection_ap_detach_retriable(conn, TO_ORIGIN_CIRCUIT(circ),
                                        END_STREAM_REASON_TIMEOUT)<0) {
-      connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
+      if (!conn->_base.marked_for_close)
+        connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
     }
   }); /* end foreach */
 }
@@ -452,7 +453,9 @@ connection_ap_attach_pending(void)
       continue;
     edge_conn = TO_EDGE_CONN(conn);
     if (connection_ap_handshake_attach_circuit(edge_conn) < 0) {
-      connection_mark_unattached_ap(edge_conn, END_STREAM_REASON_CANT_ATTACH);
+      if (!edge_conn->_base.marked_for_close)
+        connection_mark_unattached_ap(edge_conn,
+                                      END_STREAM_REASON_CANT_ATTACH);
     }
   });
 }
@@ -1501,7 +1504,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
                    conn, circ, cpath) < 0) ||
         (!circ &&
          connection_ap_handshake_attach_circuit(conn) < 0)) {
-      connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
+      if (!conn->_base.marked_for_close)
+        connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
       return -1;
     }
     return 0;
@@ -1562,7 +1566,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
         conn->_base.state = AP_CONN_STATE_CIRCUIT_WAIT;
         log_info(LD_REND, "Descriptor is here and fresh enough. Great.");
         if (connection_ap_handshake_attach_circuit(conn) < 0) {
-          connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
+          if (!conn->_base.marked_for_close)
+            connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
           return -1;
         }
       } else {
@@ -2100,7 +2105,8 @@ connection_ap_make_link(char *address, uint16_t port,
 
   /* attaching to a dirty circuit is fine */
   if (connection_ap_handshake_attach_circuit(conn) < 0) {
-    connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
+    if (!conn->_base.marked_for_close)
+      connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
     return NULL;
   }
 
diff --git a/src/or/relay.c b/src/or/relay.c
index 4bbb0b5b7..0d863cd97 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -845,7 +845,9 @@ connection_edge_process_end_not_open(
        connection_edge_end_reason_str(rh->length > 0 ? reason : -1));
   if (conn->_base.type == CONN_TYPE_AP) {
     circuit_log_path(LOG_INFO,LD_APP,circ);
-    connection_mark_unattached_ap(conn, control_reason);
+    /* need to test because of detach_retriable*/
+    if (!conn->_base.marked_for_close)
+      connection_mark_unattached_ap(conn, control_reason);
   } else {
     /* we just got an 'end', don't need to send one */
     conn->_base.edge_has_sent_end = 1;
diff --git a/src/or/rendclient.c b/src/or/rendclient.c
index 6e92ef50b..b5eed4bca 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -650,7 +650,8 @@ rend_client_desc_here(const char *query)
       if (connection_ap_handshake_attach_circuit(conn) < 0) {
         /* it will never work */
         log_warn(LD_REND,"Rendezvous attempt failed. Closing.");
-        connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
+        if (!conn->_base.marked_for_close)
+          connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
       }
     } else { /* 404, or fetch didn't get that far */
       log_notice(LD_REND,"Closing stream for '%s.onion': hidden service is "