Bug #23114: Time out circuits immediately.

This changes the purpose of circuits that are past the timeout to measurement
*as they are built*, ensuring accurate application of the timeout logic.

1 files changed, 51 insertions, 4 deletions

diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c
index 5cb90c5000..0620e045d8 100644
--- a/src/or/circuitstats.c
+++ b/src/or/circuitstats.c
@@ -37,6 +37,7 @@
 #include "rendservice.h"
 #include "statefile.h"
 #include "circuitlist.h"
+#include "circuituse.h"
 
 #undef log
 #include <math.h>
@@ -612,6 +613,35 @@ circuit_build_times_rewind_history(circuit_build_times_t *cbt, int n)
 #endif /* 0 */
 
 /**
+ * Mark this circuit as timed out, but change its purpose
+ * so that it continues to build, allowing us to measure
+ * its full build time.
+ */
+void
+circuit_build_times_mark_circ_as_measurement_only(origin_circuit_t *circ)
+{
+  control_event_circuit_status(circ,
+                               CIRC_EVENT_FAILED,
+                               END_CIRC_REASON_TIMEOUT);
+  circuit_change_purpose(TO_CIRCUIT(circ),
+                         CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT);
+  /* Record this event to check for too many timeouts
+   * in a row. This function does not record a time value yet
+   * (we do that later); it only counts the fact that we did
+   * have a timeout. We also want to avoid double-counting
+   * already "relaxed" circuits, which are counted in
+   * circuit_expire_building(). */
+  if (!circ->relaxed_timeout) {
+    int first_hop_succeeded = circ->cpath &&
+          circ->cpath->state == CPATH_STATE_OPEN;
+
+    circuit_build_times_count_timeout(
+                                 get_circuit_build_times_mutable(),
+                                 first_hop_succeeded);
+  }
+}
+
+/**
  * Perform the build time work that needs to be done when a circuit
  * completes a hop.
  *
@@ -649,12 +679,29 @@ circuit_build_times_handle_completed_hop(origin_circuit_t *circ)
     return;
   }
 
+  tor_gettimeofday(&end);
+  timediff = tv_mdiff(&circ->base_.timestamp_began, &end);
+
+  /* Check if we would have timed out already. If so, change the
+   * purpose here. But don't do any timeout handling here if there
+   * are no circuits opened yet. Save it for circuit_expire_building()
+   * (to allow it to handle timeout "relaxing" over there). */
+  if (timediff > get_circuit_build_timeout_ms() &&
+      circuit_any_opened_circuits_cached()) {
+
+    /* Circuits are allowed to last longer for measurement.
+     * Switch their purpose and wait. */
+    if (circ->base_.purpose != CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) {
+      log_info(LD_CIRC,
+               "Deciding to timeout circuit "U64_FORMAT"\n",
+               U64_PRINTF_ARG(circ->global_identifier));
+      circuit_build_times_mark_circ_as_measurement_only(circ);
+    }
+  }
+
   /* If the circuit is built to exactly the DEFAULT_ROUTE_LEN,
    * add it to our buildtimes. */
   if (circuit_get_cpath_opened_len(circ) == DEFAULT_ROUTE_LEN) {
-    tor_gettimeofday(&end);
-    timediff = tv_mdiff(&circ->base_.timestamp_began, &end);
-
     /* If the circuit build time is much greater than we would have cut
      * it off at, we probably had a suspend event along this codepath,
      * and we should discard the value.
@@ -668,7 +715,7 @@ circuit_build_times_handle_completed_hop(origin_circuit_t *circ)
     } else {
       /* Only count circuit times if the network is live */
       if (circuit_build_times_network_check_live(
-                                                 get_circuit_build_times())) {
+                            get_circuit_build_times())) {
         circuit_build_times_add_time(get_circuit_build_times_mutable(),
                                      (build_time_t)timediff);
         circuit_build_times_set_timeout(get_circuit_build_times_mutable());