Fix a bug where streams would linger forever when we had no dirinfo

fixes bug 8387; fix on 0.1.1.11-alpha (code), or on 0.2.4.10-alpha (behavior).

4 files changed, 38 insertions, 6 deletions

diff --git a/changes/bug8387 b/changes/bug8387
new file mode 100644
index 0000000000..2ec0487bf8
--- /dev/null
+++ b/changes/bug8387
@@ -0,0 +1,11 @@
+  o Major bugfixes (client):
+
+    - Perform circuit cleanup operations even when circuit
+      construction operations are disabled (because the network is
+      disabled, or because there isn't enough directory information).
+      Previously, when we were not building predictive circuits, we
+      were not closing expired circuits either.
+
+      Fixes bug 8387; bugfix on 0.1.1.11-alpha. This bug became visible
+      in 0.2.4.10-alpha when we became more strict about when we have
+      "enough directory information to build circuits".
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index c2d2b2e87c..9933b9c47e 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -969,7 +969,6 @@ circuit_predict_and_launch_new(void)
 void
 circuit_build_needed_circs(time_t now)
 {
-  static time_t time_to_new_circuit = 0;
   const or_options_t *options = get_options();
 
   /* launch a new circ for any pending streams that need one */
@@ -978,14 +977,34 @@ circuit_build_needed_circs(time_t now)
   /* make sure any hidden services have enough intro points */
   rend_services_introduce();
 
-  if (time_to_new_circuit < now) {
+  circuit_expire_old_circs_as_needed(now);
+
+  if (!options->DisablePredictedCircuits)
+    circuit_predict_and_launch_new();
+}
+
+/**
+ * Called once a second either directly or from
+ * circuit_build_needed_circs(). As appropriate (once per NewCircuitPeriod)
+ * resets failure counts and expires old circuits.
+ */
+void
+circuit_expire_old_circs_as_needed(time_t now)
+{
+  static time_t time_to_expire_and_reset = 0;
+
+  if (time_to_expire_and_reset < now) {
     circuit_reset_failure_count(1);
-    time_to_new_circuit = now + options->NewCircuitPeriod;
+    time_to_expire_and_reset = now + get_options()->NewCircuitPeriod;
     if (proxy_mode(get_options()))
       addressmap_clean(now);
     circuit_expire_old_circuits_clientside();
 
 #if 0 /* disable for now, until predict-and-launch-new can cull leftovers */
+
+    /* If we ever re-enable, this has to move into
+     * circuit_build_needed_circs */
+
     circ = circuit_get_youngest_clean_open(CIRCUIT_PURPOSE_C_GENERAL);
     if (get_options()->RunTesting &&
         circ &&
@@ -995,8 +1014,6 @@ circuit_build_needed_circs(time_t now)
     }
 #endif
   }
-  if (!options->DisablePredictedCircuits)
-    circuit_predict_and_launch_new();
 }
 
 /** If the stream <b>conn</b> is a member of any of the linked
diff --git a/src/or/circuituse.h b/src/or/circuituse.h
index 11e5a64163..1559a4070f 100644
--- a/src/or/circuituse.h
+++ b/src/or/circuituse.h
@@ -21,6 +21,7 @@ int circuit_conforms_to_options(const origin_circuit_t *circ,
                                 const or_options_t *options);
 #endif
 void circuit_build_needed_circs(time_t now);
+void circuit_expire_old_circs_as_needed(time_t now);
 void circuit_detach_stream(circuit_t *circ, edge_connection_t *conn);
 
 void circuit_expire_old_circuits_serverside(time_t now);
diff --git a/src/or/main.c b/src/or/main.c
index bd23141b97..495a71f89e 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -1485,8 +1485,11 @@ run_scheduled_events(time_t now)
    *    and we make a new circ if there are no clean circuits.
    */
   have_dir_info = router_have_minimum_dir_info();
-  if (have_dir_info && !net_is_disabled())
+  if (have_dir_info && !net_is_disabled()) {
     circuit_build_needed_circs(now);
+  } else {
+    circuit_expire_old_circs_as_needed(now);
+  }
 
   /* every 10 seconds, but not at the same second as other such events */
   if (now % 10 == 5)