From e4aaf7666028c30866ad63053ad9f6eb6bf16bf7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 21 Sep 2016 19:01:12 -0400 Subject: [PATCH 1/2] When clearing cells from a circuit for OOM reasons, tell cmux we did so. Not telling the cmux would sometimes cause an assertion failure in relay.c when we tried to get an active circuit and found an "active" circuit with no cells. Additionally, replace that assert with a test and a log message. Fix for bug 20203. This is actually probably a bugfix on 0.2.8.1-alpha, specifically my code in 8b4e5b7ee902fb7fa0776 where I made circuit_mark_for_close_() do less in order to simplify our call graph. Thanks to "cypherpunks" for help diagnosing. --- changes/bug20203 | 6 ++++++ src/or/circuitlist.c | 10 ++++++++-- src/or/relay.c | 9 +++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 changes/bug20203 diff --git a/changes/bug20203 b/changes/bug20203 new file mode 100644 index 000000000..b4809ffbd --- /dev/null +++ b/changes/bug20203 @@ -0,0 +1,6 @@ + o Major bugfixes (relay, OOM handler) + - Fix a timing-dependent assertion failure that could occur when we + tried to flush from a circuit after having freed its cells because + of an out-of-memory condition. Fixes bug 20203; bugfix on + 0.2.8.1-alpha. Thanks to "cypherpunks" for help diagnosing this + one. diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 716024df6..5fb8be54b 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1836,8 +1836,14 @@ marked_circuit_free_cells(circuit_t *circ) return; } cell_queue_clear(&circ->n_chan_cells); - if (! CIRCUIT_IS_ORIGIN(circ)) - cell_queue_clear(& TO_OR_CIRCUIT(circ)->p_chan_cells); + if (circ->n_mux) + circuitmux_clear_num_cells(circ->n_mux, circ); + if (! CIRCUIT_IS_ORIGIN(circ)) { + or_circuit_t *orcirc = TO_OR_CIRCUIT(circ); + cell_queue_clear(&orcirc->p_chan_cells); + if (orcirc->p_mux) + circuitmux_clear_num_cells(orcirc->p_mux, circ); + } } static size_t diff --git a/src/or/relay.c b/src/or/relay.c index eddad6a0c..33191e1e8 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2619,6 +2619,15 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max)) } /* Circuitmux told us this was active, so it should have cells */ + if (/*BUG(*/ queue->n == 0 /*)*/) { + log_warn(LD_BUG, "Found a supposedly active circuit with no cells " + "to send. Trying to recover."); + circuitmux_set_num_cells(cmux, circ, 0); + if (! circ->marked_for_close) + circuit_mark_for_close(circ, END_CIRC_REASON_INTERNAL); + continue; + } + tor_assert(queue->n > 0); /* From d78711c0ae83c505a966791a18a6b8d5b7e4a8d1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 22 Sep 2016 15:19:59 -0400 Subject: [PATCH 2/2] LintChanges fix --- changes/bug20203 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/bug20203 b/changes/bug20203 index b4809ffbd..711c91ba8 100644 --- a/changes/bug20203 +++ b/changes/bug20203 @@ -1,4 +1,4 @@ - o Major bugfixes (relay, OOM handler) + o Major bugfixes (relay, OOM handler): - Fix a timing-dependent assertion failure that could occur when we tried to flush from a circuit after having freed its cells because of an out-of-memory condition. Fixes bug 20203; bugfix on