Fix assertion failure spotted by arma.

Apparently, when a DNS failure was already cached, then when we tried
to mark the exit connection as closed, we'd try to remove it from the
pending queue anyway, and hit an assert.  Now, we put failed-resolve
connections in a separate state so that mark_for_close does the right
thing.


svn:r1196

3 files changed, 21 insertions, 20 deletions

diff --git a/src/or/connection.c b/src/or/connection.c
index 700c056fb..e81635a11 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -37,16 +37,18 @@ char *conn_state_to_string[][_CONN_TYPE_MAX+1] = {
   { "",                          /* exit, 0 */
     "waiting for dest info",           /* 1 */
     "connecting",                      /* 2 */
-    "open" },                          /* 3 */
+    "open"                             /* 3 */
+    "resolve failed" },                /* 4 */
   { "ready" }, /* app listener, 0 */
   { "", /* 0 */
     "", /* 1 */
     "", /* 2 */
     "", /* 3 */
-    "awaiting dest info",         /* app, 4 */
-    "waiting for safe circuit",        /* 5 */
-    "waiting for connected",           /* 6 */
-    "open" },                          /* 7 */
+    "", /* 4 */
+    "awaiting dest info",         /* app, 5 */
+    "waiting for safe circuit",        /* 6 */
+    "waiting for connected",           /* 7 */
+    "open" },                          /* 8 */
   { "ready" }, /* dir listener, 0 */
   { "",                           /* dir, 0 */
     "connecting (fetch)",              /* 1 */
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 91f9b4d20..0b25f8927 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -867,15 +867,13 @@ static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ) {
       return 0;
     case -1: /* resolve failed */
       log_fn(LOG_INFO,"Resolve failed (%s).", n_stream->address);
+      /* Set the state so that we don't try to remove n_stream from a DNS
+       * pending list. */
+      n_stream->state = EXIT_CONN_STATE_RESOLVEFAILED; 
       connection_mark_for_close(n_stream, END_STREAM_REASON_RESOLVEFAILED);
-/* XXX BUG: we're in state RESOLVING here, but we haven't been added to the
- * 'pending' list, because the dns lookup was already cached as failed.
- * But the mark_for_close will try to remove us from the pending list,
- * and we'll trigger an assert (dns.c line 209).
- * Should we add another EXIT_CONN state? Should we put an exception
- * here? Or there?
- */
-    /* case 0, resolve added to pending list */
+      break;
+    case 0: /* resolve added to pending list */
+      ;
   }
   return 0;
 }
diff --git a/src/or/or.h b/src/or/or.h
index dc513f99e..b483ed7a6 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -148,19 +148,20 @@
 #define EXIT_CONN_STATE_RESOLVING 1 /* waiting for response from dns farm */
 #define EXIT_CONN_STATE_CONNECTING 2 /* waiting for connect() to finish */
 #define EXIT_CONN_STATE_OPEN 3
-#define _EXIT_CONN_STATE_MAX 3
+#define EXIT_CONN_STATE_RESOLVEFAILED 4 /* waiting to be remoed*/
+#define _EXIT_CONN_STATE_MAX 4
 #if 0
 #define EXIT_CONN_STATE_CLOSE 3 /* flushing the buffer, then will close */
 #define EXIT_CONN_STATE_CLOSE_WAIT 4 /* have sent a destroy, awaiting a confirmation */
 #endif
 
 /* the AP state values must be disjoint from the EXIT state values */
-#define _AP_CONN_STATE_MIN 4
-#define AP_CONN_STATE_SOCKS_WAIT 4
-#define AP_CONN_STATE_CIRCUIT_WAIT 5
-#define AP_CONN_STATE_CONNECTING 6
-#define AP_CONN_STATE_OPEN 7
-#define _AP_CONN_STATE_MAX 7
+#define _AP_CONN_STATE_MIN 5
+#define AP_CONN_STATE_SOCKS_WAIT 5
+#define AP_CONN_STATE_CIRCUIT_WAIT 6
+#define AP_CONN_STATE_CONNECTING 7
+#define AP_CONN_STATE_OPEN 8
+#define _AP_CONN_STATE_MAX 8
 
 #define _DIR_CONN_STATE_MIN 1
 #define DIR_CONN_STATE_CONNECTING_FETCH 1