Merge branch 'maint-0.3.4' into release-0.3.4

15 files changed, 314 insertions, 64 deletions

diff --git a/changes/bug23512 b/changes/bug23512
new file mode 100644
index 0000000000..91b2786de4
--- /dev/null
+++ b/changes/bug23512
@@ -0,0 +1,6 @@
+  o Major bugfix (Relay bandwidth statistics):
+    - When we close relayed circuits, report the data in the circuit queues
+      as being written in our relay bandwidth stats. This mitigates guard
+      discovery and other attacks that close circuits for the explicit purpose
+      of noticing this discrepancy in statistics. Fixes bug 23512; bugfix
+      on 0.0.8pre3.
diff --git a/changes/bug25733 b/changes/bug25733
new file mode 100644
index 0000000000..775c1ae00e
--- /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/changes/bug27073 b/changes/bug27073
new file mode 100644
index 0000000000..851e7f0df6
--- /dev/null
+++ b/changes/bug27073
@@ -0,0 +1,4 @@
+  o Minor bugfixes (testing):
+    - Revise the "conditionvar_timeout" test so that it succeeds even
+      on heavily loaded systems where the test threads are not scheduled
+      within 200 msec. Fixes bug 27073; bugfix on 0.2.6.3-alpha.
diff --git a/changes/bug28096 b/changes/bug28096
new file mode 100644
index 0000000000..6847df9798
--- /dev/null
+++ b/changes/bug28096
@@ -0,0 +1,13 @@
+  o Minor bugfixes (Windows):
+    - Correctly identify Windows 8.1, Windows 10, and Windows Server 2008
+      and later from their NT versions.
+      Fixes bug 28096; bugfix on 0.2.2.34; reported by Keifer Bly.
+    - On recent Windows versions, the GetVersionEx() function may report
+      an earlier Windows version than the running OS. To avoid user
+      confusion, add "[or later]" to Tor's version string on affected
+      versions of Windows.
+      Fixes bug 28096; bugfix on 0.2.2.34; reported by Keifer Bly.
+    - Remove Windows versions that were never supported by the
+      GetVersionEx() function. Stop duplicating the latest Windows
+      version in get_uname().
+      Fixes bug 28096; bugfix on 0.2.2.34; reported by Keifer Bly.
diff --git a/src/common/compat.c b/src/common/compat.c
index 6fdd6ecf00..f8c4bdb4d8 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -2703,22 +2703,33 @@ get_uname,(void))
 #ifdef _WIN32
         OSVERSIONINFOEX info;
         int i;
+        int is_client = 0;
+        int is_server = 0;
         const char *plat = NULL;
         static struct {
-          unsigned major; unsigned minor; const char *version;
+          unsigned major; unsigned minor;
+          const char *client_version; const char *server_version;
         } win_version_table[] = {
-          { 6, 2, "Windows 8" },
-          { 6, 1, "Windows 7" },
-          { 6, 0, "Windows Vista" },
-          { 5, 2, "Windows Server 2003" },
-          { 5, 1, "Windows XP" },
-          { 5, 0, "Windows 2000" },
-          /* { 4, 0, "Windows NT 4.0" }, */
-          { 4, 90, "Windows Me" },
-          { 4, 10, "Windows 98" },
-          /* { 4, 0, "Windows 95" } */
-          { 3, 51, "Windows NT 3.51" },
-          { 0, 0, NULL }
+    /* This table must be sorted in descending order.
+     * Sources:
+     *   https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions
+     *   https://docs.microsoft.com/en-us/windows/desktop/api/winnt/
+     *     ns-winnt-_osversioninfoexa#remarks
+     */
+    /* Windows Server 2019 is indistinguishable from Windows Server 2016
+     * using GetVersionEx().
+    { 10,  0, NULL,                        "Windows Server 2019" }, */
+    { 10,  0, "Windows 10",                "Windows Server 2016" },
+    {  6,  3, "Windows 8.1",               "Windows Server 2012 R2" },
+    {  6,  2, "Windows 8",                 "Windows Server 2012" },
+    {  6,  1, "Windows 7",                 "Windows Server 2008 R2" },
+    {  6,  0, "Windows Vista",             "Windows Server 2008" },
+    {  5,  2, "Windows XP Professional",   "Windows Server 2003" },
+    /* Windows XP did not have a server version, but we need something here */
+    {  5,  1, "Windows XP",                "Windows XP Server" },
+    {  5,  0, "Windows 2000 Professional", "Windows 2000 Server" },
+    /* Earlier versions are not supported by GetVersionEx(). */
+    {  0,  0, NULL,                        NULL }
         };
         memset(&info, 0, sizeof(info));
         info.dwOSVersionInfoSize = sizeof(info);
@@ -2728,25 +2739,34 @@ get_uname,(void))
           uname_result_is_set = 1;
           return uname_result;
         }
-        if (info.dwMajorVersion == 4 && info.dwMinorVersion == 0) {
-          if (info.dwPlatformId == VER_PLATFORM_WIN32_NT)
-            plat = "Windows NT 4.0";
-          else
-            plat = "Windows 95";
+#ifdef VER_NT_SERVER
+        if (info.wProductType == VER_NT_SERVER ||
+            info.wProductType == VER_NT_DOMAIN_CONTROLLER) {
+          is_server = 1;
         } else {
-          for (i=0; win_version_table[i].major>0; ++i) {
-            if (win_version_table[i].major == info.dwMajorVersion &&
-                win_version_table[i].minor == info.dwMinorVersion) {
-              plat = win_version_table[i].version;
-              break;
+          is_client = 1;
+        }
+#endif
+        /* Search the version table for a matching version */
+        for (i=0; win_version_table[i].major>0; ++i) {
+          if (win_version_table[i].major == info.dwMajorVersion &&
+              win_version_table[i].minor == info.dwMinorVersion) {
+            if (is_server) {
+              plat = win_version_table[i].server_version;
+            } else {
+              /* Use client versions for clients, and when we don't know if it
+              * is a client or a server. */
+              plat = win_version_table[i].client_version;
             }
+            break;
           }
         }
         if (plat) {
           strlcpy(uname_result, plat, sizeof(uname_result));
         } else {
-          if (info.dwMajorVersion > 6 ||
-              (info.dwMajorVersion==6 && info.dwMinorVersion>2))
+          if (info.dwMajorVersion > win_version_table[0].major ||
+              (info.dwMajorVersion == win_version_table[0].major &&
+               info.dwMinorVersion > win_version_table[0].minor))
             tor_snprintf(uname_result, sizeof(uname_result),
                          "Very recent version of Windows [major=%d,minor=%d]",
                          (int)info.dwMajorVersion,(int)info.dwMinorVersion);
@@ -2755,12 +2775,25 @@ get_uname,(void))
                          "Unrecognized version of Windows [major=%d,minor=%d]",
                          (int)info.dwMajorVersion,(int)info.dwMinorVersion);
         }
-#ifdef VER_NT_SERVER
-      if (info.wProductType == VER_NT_SERVER ||
-          info.wProductType == VER_NT_DOMAIN_CONTROLLER) {
-        strlcat(uname_result, " [server]", sizeof(uname_result));
-      }
-#endif /* defined(VER_NT_SERVER) */
+        /* Now append extra information to the name.
+         *
+         * Microsoft's API documentation says that on Windows 8.1 and later,
+         * GetVersionEx returns Windows 8 (6.2) for applications without an
+         * app compatibility manifest (including tor's default build).
+         *
+         * But in our testing, we have seen the actual Windows version on
+         * Windows Server 2012 R2, even without a manifest. */
+        if (info.dwMajorVersion > 6 ||
+            (info.dwMajorVersion == 6 && info.dwMinorVersion >= 2)) {
+          /* When GetVersionEx() returns Windows 8, the actual OS may be any
+           * later version. */
+          strlcat(uname_result, " [or later]", sizeof(uname_result));
+        }
+        /* When we don't know if the OS is a client or server version, we use
+         * the client version, and this qualifier. */
+        if (!is_server && !is_client) {
+          strlcat(uname_result, " [client or server]", sizeof(uname_result));
+        }
 #else /* !(defined(_WIN32)) */
         /* LCOV_EXCL_START -- can't provoke uname failure */
         strlcpy(uname_result, "Unknown platform", sizeof(uname_result));
diff --git a/src/or/channeltls.h b/src/or/channeltls.h
index d9c4239c3a..65ce387a9a 100644
--- a/src/or/channeltls.h
+++ b/src/or/channeltls.h
@@ -12,6 +12,8 @@
 #include "or.h"
 #include "channel.h"
 
+#define TLS_PER_CELL_OVERHEAD 29
+
 #define BASE_CHAN_TO_TLS(c) (channel_tls_from_base((c)))
 #define TLS_CHAN_TO_BASE(c) (channel_tls_to_base((c)))
 
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index ad9b902ac6..fdb039ef0d 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -55,6 +55,7 @@
 
 #include "or.h"
 #include "channel.h"
+#include "channeltls.h"
 #include "circpathbias.h"
 #include "circuitbuild.h"
 #include "circuitlist.h"
@@ -1994,6 +1995,61 @@ circuit_mark_all_dirty_circs_as_unusable(void)
   SMARTLIST_FOREACH_END(circ);
 }
 
+/**
+ * Report any queued cells on or_circuits as written in our bandwidth
+ * totals, for the specified channel direction.
+ *
+ * When we close a circuit or clear its cell queues, we've read
+ * data and recorded those bytes in our read statistics, but we're
+ * not going to write it. This discrepancy can be used by an adversary
+ * to infer information from our public relay statistics and perform
+ * attacks such as guard discovery.
+ *
+ * This function is in the critical path of circuit_mark_for_close().
+ * It must be (and is) O(1)!
+ *
+ * See https://trac.torproject.org/projects/tor/ticket/23512.
+ */
+void
+circuit_synchronize_written_or_bandwidth(const circuit_t *c,
+                                         circuit_channel_direction_t dir)
+{
+  uint64_t cells;
+  uint64_t cell_size;
+  uint64_t written_sync;
+  const channel_t *chan = NULL;
+  const or_circuit_t *or_circ;
+
+  if (!CIRCUIT_IS_ORCIRC(c))
+    return;
+
+  or_circ = CONST_TO_OR_CIRCUIT(c);
+
+  if (dir == CIRCUIT_N_CHAN) {
+    chan = c->n_chan;
+    cells = c->n_chan_cells.n;
+  } else {
+    chan = or_circ->p_chan;
+    cells = or_circ->p_chan_cells.n;
+  }
+
+  /* If we still know the chan, determine real cell size. Otherwise,
+   * assume it's a wide circid channel */
+  if (chan)
+    cell_size = get_cell_network_size(chan->wide_circ_ids);
+  else
+    cell_size = CELL_MAX_NETWORK_SIZE;
+
+  /* The missing written bytes are the cell counts times their cell
+   * size plus TLS per cell overhead */
+  written_sync = cells*(cell_size+TLS_PER_CELL_OVERHEAD);
+
+  /* Report the missing bytes as written, to avoid asymmetry.
+   * We must use time() for consistency with rephist, even though on
+   * some very old rare platforms, approx_time() may be faster. */
+  rep_hist_note_bytes_written(written_sync, time(NULL));
+}
+
 /** Mark <b>circ</b> to be closed next time we call
  * circuit_close_all_marked(). Do any cleanup needed:
  *   - If state is onionskin_pending, remove circ from the onion_pending
@@ -2045,6 +2101,9 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line,
     reason = END_CIRC_REASON_NONE;
   }
 
+  circuit_synchronize_written_or_bandwidth(circ, CIRCUIT_N_CHAN);
+  circuit_synchronize_written_or_bandwidth(circ, CIRCUIT_P_CHAN);
+
   if (reason & END_CIRC_REASON_FLAG_REMOTE)
     reason &= ~END_CIRC_REASON_FLAG_REMOTE;
 
diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h
index 246f0c8815..1d39f36925 100644
--- a/src/or/circuitlist.h
+++ b/src/or/circuitlist.h
@@ -68,6 +68,8 @@ crypt_path_t *circuit_get_cpath_hop(origin_circuit_t *circ, int hopnum);
 void circuit_get_all_pending_on_channel(smartlist_t *out,
                                         channel_t *chan);
 int circuit_count_pending_on_channel(channel_t *chan);
+void circuit_synchronize_written_or_bandwidth(const circuit_t *c,
+                                              circuit_channel_direction_t dir);
 
 #define circuit_mark_for_close(c, reason)                               \
   circuit_mark_for_close_((c), (reason), __LINE__, SHORT_FILE__)
diff --git a/src/or/or.h b/src/or/or.h
index 98b7fc9772..0de07ed7ab 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -3056,6 +3056,18 @@ typedef struct testing_cell_stats_entry_t {
 } testing_cell_stats_entry_t;
 
 /**
+ * An enum to allow us to specify which channel in a circuit
+ * we're interested in.
+ *
+ * This is needed because our data structures and other fields
+ * for channel delivery are disassociated from the channel.
+ */
+typedef enum {
+  CIRCUIT_N_CHAN = 0,
+  CIRCUIT_P_CHAN = 1
+} circuit_channel_direction_t;
+
+/**
  * A circuit is a path over the onion routing
  * network. Applications can connect to one end of the circuit, and can
  * create exit connections at the other end of the circuit. AP and exit
diff --git a/src/or/relay.c b/src/or/relay.c
index 497afe756a..b6886e2bb8 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -1729,6 +1729,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       }
       if (circ->n_chan) {
         uint8_t trunc_reason = get_uint8(cell->payload + RELAY_HEADER_SIZE);
+        circuit_synchronize_written_or_bandwidth(circ, CIRCUIT_N_CHAN);
         circuit_clear_cell_queue(circ, circ->n_chan);
         channel_send_destroy(circ->n_circ_id, circ->n_chan,
                              trunc_reason);
diff --git a/src/or/rephist.c b/src/or/rephist.c
index f86736e219..aa31ef973c 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -92,6 +92,11 @@
 static void bw_arrays_init(void);
 static void predicted_ports_alloc(void);
 
+typedef struct bw_array_t bw_array_t;
+STATIC uint64_t find_largest_max(bw_array_t *b);
+STATIC void commit_max(bw_array_t *b);
+STATIC void advance_obs(bw_array_t *b);
+
 /** Total number of bytes currently allocated in fields used by rephist.c. */
 uint64_t rephist_total_alloc=0;
 /** Number of or_history_t objects currently allocated. */
@@ -979,7 +984,7 @@ rep_hist_load_mtbf_data(time_t now)
 /** Structure to track bandwidth use, and remember the maxima for a given
  * time period.
  */
-typedef struct bw_array_t {
+struct bw_array_t {
   /** Observation array: Total number of bytes transferred in each of the last
    * NUM_SECS_ROLLING_MEASURE seconds. This is used as a circular array. */
   uint64_t obs[NUM_SECS_ROLLING_MEASURE];
@@ -1006,10 +1011,10 @@ typedef struct bw_array_t {
   /** Circular array of the total bandwidth usage for the last NUM_TOTALS
    * periods */
   uint64_t totals[NUM_TOTALS];
-} bw_array_t;
+};
 
 /** Shift the current period of b forward by one. */
-static void
+STATIC void
 commit_max(bw_array_t *b)
 {
   /* Store total from current period. */
@@ -1029,7 +1034,7 @@ commit_max(bw_array_t *b)
 }
 
 /** Shift the current observation time of <b>b</b> forward by one second. */
-static inline void
+STATIC void
 advance_obs(bw_array_t *b)
 {
   int nextidx;
@@ -1107,7 +1112,7 @@ bw_array_free_(bw_array_t *b)
 /** Recent history of bandwidth observations for read operations. */
 static bw_array_t *read_array = NULL;
 /** Recent history of bandwidth observations for write operations. */
-static bw_array_t *write_array = NULL;
+STATIC bw_array_t *write_array = NULL;
 /** Recent history of bandwidth observations for read operations for the
     directory protocol. */
 static bw_array_t *dir_read_array = NULL;
@@ -1139,7 +1144,7 @@ bw_arrays_init(void)
  * earlier than the latest <b>when</b> you've heard of.
  */
 void
-rep_hist_note_bytes_written(size_t num_bytes, time_t when)
+rep_hist_note_bytes_written(uint64_t num_bytes, time_t when)
 {
 /* Maybe a circular array for recent seconds, and step to a new point
  * every time a new second shows up. Or simpler is to just to have
@@ -1156,7 +1161,7 @@ rep_hist_note_bytes_written(size_t num_bytes, time_t when)
  * (like rep_hist_note_bytes_written() above)
  */
 void
-rep_hist_note_bytes_read(size_t num_bytes, time_t when)
+rep_hist_note_bytes_read(uint64_t num_bytes, time_t when)
 {
 /* if we're smart, we can make this func and the one above share code */
   add_obs(read_array, when, num_bytes);
@@ -1166,7 +1171,7 @@ rep_hist_note_bytes_read(size_t num_bytes, time_t when)
  * <b>when</b>. (like rep_hist_note_bytes_written() above)
  */
 void
-rep_hist_note_dir_bytes_written(size_t num_bytes, time_t when)
+rep_hist_note_dir_bytes_written(uint64_t num_bytes, time_t when)
 {
   add_obs(dir_write_array, when, num_bytes);
 }
@@ -1175,7 +1180,7 @@ rep_hist_note_dir_bytes_written(size_t num_bytes, time_t when)
  * <b>when</b>. (like rep_hist_note_bytes_written() above)
  */
 void
-rep_hist_note_dir_bytes_read(size_t num_bytes, time_t when)
+rep_hist_note_dir_bytes_read(uint64_t num_bytes, time_t when)
 {
   add_obs(dir_read_array, when, num_bytes);
 }
@@ -1184,7 +1189,7 @@ rep_hist_note_dir_bytes_read(size_t num_bytes, time_t when)
  * most bandwidth used in any NUM_SECS_ROLLING_MEASURE period for the last
  * NUM_SECS_BW_SUM_IS_VALID seconds.)
  */
-static uint64_t
+STATIC uint64_t
 find_largest_max(bw_array_t *b)
 {
   int i;
diff --git a/src/or/rephist.h b/src/or/rephist.h
index 3e64a3de40..9ef49d19c0 100644
--- a/src/or/rephist.h
+++ b/src/or/rephist.h
@@ -14,13 +14,13 @@
 
 void rep_hist_init(void);
 void rep_hist_dump_stats(time_t now, int severity);
-void rep_hist_note_bytes_read(size_t num_bytes, time_t when);
-void rep_hist_note_bytes_written(size_t num_bytes, time_t when);
+void rep_hist_note_bytes_read(uint64_t num_bytes, time_t when);
+void rep_hist_note_bytes_written(uint64_t num_bytes, time_t when);
 
 void rep_hist_make_router_pessimal(const char *id, time_t when);
 
-void rep_hist_note_dir_bytes_read(size_t num_bytes, time_t when);
-void rep_hist_note_dir_bytes_written(size_t num_bytes, time_t when);
+void rep_hist_note_dir_bytes_read(uint64_t num_bytes, time_t when);
+void rep_hist_note_dir_bytes_written(uint64_t num_bytes, time_t when);
 
 MOCK_DECL(int, rep_hist_bandwidth_assess, (void));
 char *rep_hist_get_bandwidth_lines(void);
@@ -109,6 +109,8 @@ extern uint32_t rephist_total_num;
 #ifdef TOR_UNIT_TESTS
 extern int onion_handshakes_requested[MAX_ONION_HANDSHAKE_TYPE+1];
 extern int onion_handshakes_assigned[MAX_ONION_HANDSHAKE_TYPE+1];
+typedef struct bw_array_t bw_array_t;
+extern bw_array_t *write_array;
 #endif
 
 /**
diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c
index af8ddccabd..e941626371 100644
--- a/src/or/scheduler_kist.c
+++ b/src/or/scheduler_kist.c
@@ -14,8 +14,6 @@
 #define SCHEDULER_PRIVATE_
 #include "scheduler.h"
 
-#define TLS_PER_CELL_OVERHEAD 29
-
 #ifdef HAVE_KIST_SUPPORT
 /* Kernel interface needed for KIST. */
 #include <netinet/tcp.h>
diff --git a/src/test/test_relay.c b/src/test/test_relay.c
index 73c0ed5586..a64d9af5b9 100644
--- a/src/test/test_relay.c
+++ b/src/test/test_relay.c
@@ -4,6 +4,9 @@
 #include "or.h"
 #define CIRCUITBUILD_PRIVATE
 #include "circuitbuild.h"
+#include "circuitlist.h"
+#include "rephist.h"
+#include "channeltls.h"
 #define RELAY_PRIVATE
 #include "relay.h"
 /* For init/free stuff */
@@ -16,6 +19,9 @@
 static or_circuit_t * new_fake_orcirc(channel_t *nchan, channel_t *pchan);
 
 static void test_relay_append_cell_to_circuit_queue(void *arg);
+uint64_t find_largest_max(bw_array_t *b);
+void commit_max(bw_array_t *b);
+void advance_obs(bw_array_t *b);
 
 static or_circuit_t *
 new_fake_orcirc(channel_t *nchan, channel_t *pchan)
@@ -27,10 +33,9 @@ new_fake_orcirc(channel_t *nchan, channel_t *pchan)
   circ = &(orcirc->base_);
   circ->magic = OR_CIRCUIT_MAGIC;
 
-  circ->n_chan = nchan;
-  circ->n_circ_id = get_unique_circ_id_by_chan(nchan);
-  circ->n_mux = NULL; /* ?? */
+  circuit_set_n_circid_chan(circ, get_unique_circ_id_by_chan(nchan), nchan);
   cell_queue_init(&(circ->n_chan_cells));
+
   circ->n_hop = NULL;
   circ->streams_blocked_on_n_chan = 0;
   circ->streams_blocked_on_p_chan = 0;
@@ -43,14 +48,109 @@ new_fake_orcirc(channel_t *nchan, channel_t *pchan)
   circ->deliver_window = CIRCWINDOW_START_MAX;
   circ->n_chan_create_cell = NULL;
 
-  orcirc->p_chan = pchan;
-  orcirc->p_circ_id = get_unique_circ_id_by_chan(pchan);
+  circuit_set_p_circid_chan(orcirc, get_unique_circ_id_by_chan(pchan), pchan);
   cell_queue_init(&(orcirc->p_chan_cells));
 
   return orcirc;
 }
 
 static void
+assert_circuit_ok_mock(const circuit_t *c)
+{
+  (void) c;
+  return;
+}
+
+static void
+test_relay_close_circuit(void *arg)
+{
+  channel_t *nchan = NULL, *pchan = NULL;
+  or_circuit_t *orcirc = NULL;
+  cell_t *cell = NULL;
+  int old_count, new_count;
+
+  (void)arg;
+
+  /* Make fake channels to be nchan and pchan for the circuit */
+  nchan = new_fake_channel();
+  tt_assert(nchan);
+
+  pchan = new_fake_channel();
+  tt_assert(pchan);
+
+  /* Make a fake orcirc */
+  orcirc = new_fake_orcirc(nchan, pchan);
+  tt_assert(orcirc);
+  circuitmux_attach_circuit(nchan->cmux, TO_CIRCUIT(orcirc),
+                            CELL_DIRECTION_OUT);
+  circuitmux_attach_circuit(pchan->cmux, TO_CIRCUIT(orcirc),
+                            CELL_DIRECTION_IN);
+
+  /* Make a cell */
+  cell = tor_malloc_zero(sizeof(cell_t));
+  make_fake_cell(cell);
+
+  MOCK(scheduler_channel_has_waiting_cells,
+       scheduler_channel_has_waiting_cells_mock);
+  MOCK(assert_circuit_ok,
+       assert_circuit_ok_mock);
+
+  /* Append it */
+  old_count = get_mock_scheduler_has_waiting_cells_count();
+  append_cell_to_circuit_queue(TO_CIRCUIT(orcirc), nchan, cell,
+                               CELL_DIRECTION_OUT, 0);
+  new_count = get_mock_scheduler_has_waiting_cells_count();
+  tt_int_op(new_count, OP_EQ, old_count + 1);
+
+  /* Now try the reverse direction */
+  old_count = get_mock_scheduler_has_waiting_cells_count();
+  append_cell_to_circuit_queue(TO_CIRCUIT(orcirc), pchan, cell,
+                               CELL_DIRECTION_IN, 0);
+  new_count = get_mock_scheduler_has_waiting_cells_count();
+  tt_int_op(new_count, OP_EQ, old_count + 1);
+
+  /* Ensure our write totals are 0 */
+  tt_u64_op(find_largest_max(write_array), OP_EQ, 0);
+
+  /* Mark the circuit for close */
+  circuit_mark_for_close(TO_CIRCUIT(orcirc), 0);
+
+  /* Check our write totals. */
+  advance_obs(write_array);
+  commit_max(write_array);
+  /* Check for two cells plus overhead */
+  tt_u64_op(find_largest_max(write_array), OP_EQ,
+                             2*(get_cell_network_size(nchan->wide_circ_ids)
+                                +TLS_PER_CELL_OVERHEAD));
+
+  UNMOCK(scheduler_channel_has_waiting_cells);
+
+  /* Get rid of the fake channels */
+  MOCK(scheduler_release_channel, scheduler_release_channel_mock);
+  channel_mark_for_close(nchan);
+  channel_mark_for_close(pchan);
+  UNMOCK(scheduler_release_channel);
+
+  /* Shut down channels */
+  channel_free_all();
+
+ done:
+  tor_free(cell);
+  if (orcirc) {
+    circuitmux_detach_circuit(nchan->cmux, TO_CIRCUIT(orcirc));
+    circuitmux_detach_circuit(pchan->cmux, TO_CIRCUIT(orcirc));
+    cell_queue_clear(&orcirc->base_.n_chan_cells);
+    cell_queue_clear(&orcirc->p_chan_cells);
+  }
+  tor_free(orcirc);
+  free_fake_channel(nchan);
+  free_fake_channel(pchan);
+  UNMOCK(assert_circuit_ok);
+
+  return;
+}
+
+static void
 test_relay_append_cell_to_circuit_queue(void *arg)
 {
   channel_t *nchan = NULL, *pchan = NULL;
@@ -125,6 +225,7 @@ test_relay_append_cell_to_circuit_queue(void *arg)
 struct testcase_t relay_tests[] = {
   { "append_cell_to_circuit_queue", test_relay_append_cell_to_circuit_queue,
     TT_FORK, NULL, NULL },
+  { "close_circ_rephist", test_relay_close_circuit,
+    TT_FORK, NULL, NULL },
   END_OF_TESTCASES
 };
-
diff --git a/src/test/test_threads.c b/src/test/test_threads.c
index ed6d8f04aa..4f37a40cc7 100644
--- a/src/test/test_threads.c
+++ b/src/test/test_threads.c
@@ -234,25 +234,33 @@ test_threads_conditionvar(void *arg)
   if (timeout) {
     ti->tv = &msec100;
   }
+
+#define SPIN_UNTIL(condition,sleep_msec)        \
+  while (1) {                                   \
+    tor_mutex_acquire(ti->mutex);               \
+    if (condition) {                            \
+      break;                                    \
+    }                                           \
+    tor_mutex_release(ti->mutex);               \
+    tor_sleep_msec(sleep_msec);                 \
+  }
+
   spawn_func(cv_test_thr_fn_, ti);
   spawn_func(cv_test_thr_fn_, ti);
   spawn_func(cv_test_thr_fn_, ti);
   spawn_func(cv_test_thr_fn_, ti);
 
-  tor_mutex_acquire(ti->mutex);
+  SPIN_UNTIL(ti->n_threads == 4, 10);
+
+  time_t started_at = time(NULL);
+
   ti->addend = 7;
   ti->shutdown = 1;
   tor_cond_signal_one(ti->cond);
   tor_mutex_release(ti->mutex);
 
 #define SPIN()                                  \
-  while (1) {                                   \
-    tor_mutex_acquire(ti->mutex);               \
-    if (ti->addend == 0) {                      \
-      break;                                    \
-    }                                           \
-    tor_mutex_release(ti->mutex);               \
-  }
+  SPIN_UNTIL(ti->addend == 0, 0)
 
   SPIN();
 
@@ -279,8 +287,9 @@ test_threads_conditionvar(void *arg)
   if (!timeout) {
     tt_int_op(ti->n_shutdown, OP_EQ, 4);
   } else {
-    tor_sleep_msec(200);
-    tor_mutex_acquire(ti->mutex);
+    const int GIVE_UP_AFTER_SEC = 30;
+    SPIN_UNTIL((ti->n_timeouts == 2 ||
+                time(NULL) >= started_at + GIVE_UP_AFTER_SEC), 10);
     tt_int_op(ti->n_shutdown, OP_EQ, 2);
     tt_int_op(ti->n_timeouts, OP_EQ, 2);
     tor_mutex_release(ti->mutex);
@@ -301,4 +310,3 @@ struct testcase_t thread_tests[] = {
     &passthrough_setup, (void*)"tv" },
   END_OF_TESTCASES
 };
-