r11797@catbus: nickm | 2007-02-13 16:17:40 -0500

Fix a longstanding dnsworker-related bug: when we decide to kill a DNS worker for having been in use too long, and we independently decide to close one of the exit connections waiting for that DNS worker (perhaps because we got an END relay cell), it was possible for us to try to double-free the connection.  We were smart enough to detect this and assert, but not smart enough to fix this bug before now.  This should resolve bug 390.  Backport candidate.


svn:r9581

ChangeLog

@@ -2,6 +2,8 @@ Changes in version 0.1.2.8-alpha - 2007-??-??
   o Major bugfixes (accounting):
     - When we start during an accounting interval before it's time to wake
       up, remember to wake up at the correct time. (May fix bug 342.)
+    - Fix a longstanding obscure crash bug that could occur when
+      low on DNS resources. (Resolves bug 390.)
 
   o Major bugfixes (crashes):
     - Stop crashing when the controller asks us to resetconf more than

doc/TODO

@@ -232,7 +232,7 @@ P - Figure out why openssl 0.9.8d "make test" fails at sha256t test.
       - Then clients can choose a bucket (or set of buckets) to
         download and use.
 
-  - Improvements to versioning.
+  - Improvements to versioning. (Proposal 105)
     - When we connect to a Tor server, it sends back a cell listing
       the IP it believes it is using. Use this to block dvorak's attack.
       Also, this is a fine time to say what time you think it is.

src/or/connection_edge.c

@@ -175,8 +175,8 @@ connection_edge_destroy(uint16_t circ_id, edge_connection_t *conn)
 
 /** Send a relay end cell from stream <b>conn</b> to conn's circuit,
  * with a destination of cpath_layer. (If cpath_layer is NULL, the
- * destination is the circuit's origin.) Mark the relay end cell as
- * closing because of <b>reason</b>.
+ * destination is the circuit's origin.) Set the relay end cell's
+ * reason for closing as <b>reason</b>.
  *
  * Return -1 if this function has already been called on this conn,
  * else return 0.
@@ -213,11 +213,11 @@ connection_edge_end(edge_connection_t *conn, char reason,
 
   circ = circuit_get_by_edge_conn(conn);
   if (circ && !circ->marked_for_close) {
-    log_debug(LD_EDGE,"Marking conn (fd %d) and sending end.",conn->_base.s);
+    log_debug(LD_EDGE,"Sending end on conn (fd %d).",conn->_base.s);
     connection_edge_send_command(conn, circ, RELAY_COMMAND_END,
                                  payload, payload_len, cpath_layer);
   } else {
-    log_debug(LD_EDGE,"Marking conn (fd %d); no circ to send end.",
+    log_debug(LD_EDGE,"No circ to send end on conn (fd %d).",
               conn->_base.s);
   }
 

src/or/dns.c

@@ -868,7 +868,8 @@ dns_cancel_pending_resolve(const char *address)
     circ = circuit_get_by_edge_conn(pendconn);
     if (circ)
       circuit_detach_stream(circ, pendconn);
-    connection_free(TO_CONN(pendconn));
+    if (!pendconn->_base.marked_for_close)
+      connection_free(TO_CONN(pendconn));
     resolve->pending_connections = pend->next;
     tor_free(pend);
   }