Separate failure-count tracking from circuit-launching.

Increment failure counts only when circuits close without having been built.
Reset failure counts only on the second, and when circuits are done building.


svn:r847

4 files changed, 32 insertions, 19 deletions

diff --git a/src/or/circuit.c b/src/or/circuit.c
index 09eb30b541..8641540348 100644
--- a/src/or/circuit.c
+++ b/src/or/circuit.c
@@ -566,6 +566,10 @@ void circuit_close(circuit_t *circ) {
   for(conn=circ->p_streams; conn; conn=conn->next_stream) {
     connection_send_destroy(circ->p_circ_id, conn); 
   }
+  if (circ->state != CIRCUIT_STATE_OPEN && circ->cpath) {
+    /* If we never built the circuit, note it as a failure. */
+    circuit_increment_failure_count();
+  }
   circuit_free(circ);
 }
 
@@ -686,37 +690,42 @@ void circuit_expire_unused_circuits(void) {
   }
 }
 
+/* Number of failures so far this second; should only be touched by 
+ * circuit_launch_new and circuit_*_failure_count.
+ */ 
+static int n_circuit_failures = 0; 
+
 /* failure_status code: negative means reset failures to 0. Other values mean
  * add that value to the current number of failures, then if we don't have too
  * many failures on record, try to make a new circuit.
  *
  * Return -1 if you aren't going to try to make a circuit, 0 if you did try.
  */
-int circuit_launch_new(int failure_status) {
-  static int failures=0;
+int circuit_launch_new(void) {
 
   if(!options.SocksPort) /* we're not an application proxy. no need for circuits. */
     return -1;
 
-  if(failure_status == -1) { /* I was called because a circuit succeeded */
-    failures = 0;
-    return -1;
-  }
-
-  failures += failure_status;
-
-  if(failures > 5) {
+  if(n_circuit_failures > 5) {
     return -1;
   }
 
   if(circuit_establish_circuit() < 0) {
+    ++n_circuit_failures;
     return 0;
   }
 
-  failures = 0;
   return 0;
 }
 
+void circuit_increment_failure_count(void) {
+  ++n_circuit_failures;
+}
+
+void circuit_reset_failure_count(void) {
+  n_circuit_failures = 0;
+}
+
 int circuit_establish_circuit(void) {
   routerinfo_t *firsthop;
   connection_t *n_conn;
@@ -840,6 +849,7 @@ int circuit_send_next_onion_skin(circuit_t *circ) {
       /* done building the circuit. whew. */
       circ->state = CIRCUIT_STATE_OPEN;
       log_fn(LOG_INFO,"circuit built!");
+      circuit_reset_failure_count();
       /* Tell any AP connections that have been waiting for a new
        * circuit that one is ready. */
       connection_ap_attach_pending();
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index d28d6c1b26..50e612f176 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -257,7 +257,7 @@ int connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, connection
         client_dns_set_entry(conn->socks_request->address, addr);
         conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
         if(connection_ap_handshake_attach_circuit(conn) < 0)
-          circuit_launch_new(1); /* Build another circuit to handle this stream */
+          circuit_launch_new(); /* Build another circuit to handle this stream */
         return 0;
       }
       log_fn(LOG_INFO,"end cell (%s) for stream %d. Removing stream.",
@@ -499,7 +499,7 @@ void connection_ap_attach_pending(void)
     if (connection_ap_handshake_attach_circuit(carray[i])<0) {
       if(!circuit_get_newest(carray[i], 0)) {
         /* if there are no acceptable clean or not-very-dirty circs on the way */
-        circuit_launch_new(1);
+        circuit_launch_new();
       }
     }
   }
@@ -558,7 +558,7 @@ static int connection_ap_handshake_process_socks(connection_t *conn) {
 
   conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
   if(connection_ap_handshake_attach_circuit(conn) < 0)
-    circuit_launch_new(1); /* Build another circuit to handle this stream */
+    circuit_launch_new(); /* Build another circuit to handle this stream */
   return 0;
 }
 
diff --git a/src/or/main.c b/src/or/main.c
index b362b0baf5..a19079ddfd 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -356,19 +356,20 @@ static void run_scheduled_events(time_t now) {
     if(time_to_new_circuit < now) {
       client_dns_clean();
       circuit_expire_unused_circuits();
-      circuit_launch_new(-1); /* tell it to forget about previous failures */
+      circuit_reset_failure_count();
       if(circ && circ->timestamp_dirty) {
         log_fn(LOG_INFO,"Youngest circuit dirty; launching replacement.");
-        circuit_launch_new(0); /* make a new circuit */
+        circuit_launch_new(); /* make a new circuit */
       }
       time_to_new_circuit = now + options.NewCircuitPeriod;
     }
 #define CIRCUIT_MIN_BUILDING 2
-    if(!circ && circuit_count_building() < CIRCUIT_MIN_BUILDING)
+    if(!circ && circuit_count_building() < CIRCUIT_MIN_BUILDING) {
       /* if there's no open circ, and less than 2 are on the way,
        * go ahead and try another.
        */
-      circuit_launch_new(1);
+      circuit_launch_new();
+    }
   }
 
   /* 4. Every second, we check how much bandwidth we've consumed and 
diff --git a/src/or/or.h b/src/or/or.h
index 237a6d7281..b677d2d955 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -535,7 +535,9 @@ void circuit_about_to_close_connection(connection_t *conn);
 void circuit_dump_by_conn(connection_t *conn, int severity);
 
 void circuit_expire_unused_circuits(void);
-int circuit_launch_new(int failure_status);
+int circuit_launch_new(void);
+void circuit_increment_failure_count(void);
+void circuit_reset_failure_count(void);
 int circuit_establish_circuit(void);
 void circuit_n_conn_open(connection_t *or_conn);
 int circuit_send_next_onion_skin(circuit_t *circ);