[tor-commits] [tor/maint-0.3.5] Bug 25733: Avoid assert failure if all circuits time out.

[teor at torproject.org]

commit e8d84b18aabf6a3d32a41700142048224e83ed8b
Author: Mike Perry <mikeperry-git at torproject.org>
Date:   Mon Apr 16 22:49:27 2018 +0000

    Bug 25733: Avoid assert failure if all circuits time out.
    
    Prior to #23100, we were not counting HS circuit build times in our
    calculation of the timeout. This could lead to a condition where our timeout
    was set too low, based on non HS circuit build times, and then we would
    abandon all HS circuits, storing no valid timeouts in the histogram.
    
    This commit avoids the assert.
---
 changes/bug25733      |  4 ++++
 src/or/circuitstats.c | 20 ++++++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/changes/bug25733 b/changes/bug25733
new file mode 100644
index 000000000..775c1ae00
--- /dev/null
+++ b/changes/bug25733
@@ -0,0 +1,4 @@
+  o Minor bugfixes (Assert crash):
+    - Avoid an assert in the circuit build timeout code if we fail to
+      allow any circuits to actually complete. Fixes bug 25733;
+      bugfix on 0.2.2.2-alpha.
diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c
index 418acc002..735b3f0fc 100644
--- a/src/or/circuitstats.c
+++ b/src/or/circuitstats.c
@@ -753,11 +753,23 @@ circuit_build_times_get_xm(circuit_build_times_t *cbt)
              histogram[nth_max_bin[n]]);
   }
 
-  /* The following assert is safe, because we don't get called when we
-   * haven't observed at least CBT_MIN_MIN_CIRCUITS_TO_OBSERVE circuits. */
+  /* bin_counts can become zero if all of our last CBT_NCIRCUITS_TO_OBSERVE
+   * circuits were abandoned before they completed. This shouldn't happen,
+   * though. We should have reset/re-learned a lower timeout first. */
+  if (bin_counts == 0) {
+    ret = 0;
+    log_warn(LD_CIRC,
+               "No valid circuit build time data out of %d times, %u modes, "
+               "have_timeout=%d, %lfms", cbt->total_build_times, num_modes,
+               cbt->have_computed_timeout, cbt->timeout_ms);
+    goto done;
+  }
+
   tor_assert(bin_counts > 0);
 
   ret /= bin_counts;
+
+ done:
   tor_free(histogram);
   tor_free(nth_max_bin);
 
@@ -1043,6 +1055,10 @@ circuit_build_times_update_alpha(circuit_build_times_t *cbt)
    * and less frechet-like. */
   cbt->Xm = circuit_build_times_get_xm(cbt);
 
+  /* If Xm came back 0, then too many circuits were abandoned. */
+  if (cbt->Xm == 0)
+    return 0;
+
   tor_assert(cbt->Xm > 0);
 
   for (i=0; i< CBT_NCIRCUITS_TO_OBSERVE; i++) {