diff --git a/changes/ticket40870 b/changes/ticket40870 new file mode 100644 index 0000000000..c33c83e1a6 --- /dev/null +++ b/changes/ticket40870 @@ -0,0 +1,4 @@ + o Minor bugfixes (conflux, client): + - Avoid a non fatal assert caused by data coming in on a conflux set that is + being freed during shutdown. Fixes bug 40870; bugfix on 0.4.8.1-alpha. + diff --git a/src/app/main/shutdown.c b/src/app/main/shutdown.c index b3f1c6d058..7f0d112c90 100644 --- a/src/app/main/shutdown.c +++ b/src/app/main/shutdown.c @@ -120,6 +120,7 @@ tor_free_all(int postfork) dirserv_free_all(); rep_hist_free_all(); bwhist_free_all(); + conflux_notify_shutdown(); circuit_free_all(); conflux_pool_free_all(); circpad_machines_free(); diff --git a/src/core/or/conflux_pool.c b/src/core/or/conflux_pool.c index 4a7e941372..ec2938aefc 100644 --- a/src/core/or/conflux_pool.c +++ b/src/core/or/conflux_pool.c @@ -1607,7 +1607,22 @@ linked_circuit_free(circuit_t *circ, bool is_client) /* Circuit can be freed without being closed and so we try to delete this leg * so we can learn if this circuit is the last leg or not. */ - cfx_del_leg(circ->conflux, circ); + if (cfx_del_leg(circ->conflux, circ)) { + /* Check for instances of bug #40870, which we suspect happen + * during exit. If any happen outside of exit, BUG and warn. */ + if (!circ->conflux->in_full_teardown) { + /* We should bug and warn if we're not in a shutdown process; that + * means we got here somehow without a close. */ + if (BUG(!shutting_down)) { + log_warn(LD_BUG, + "Conflux circuit %p being freed without being marked for " + "full teardown via close, with shutdown state %d. " + "Please report this.", circ, shutting_down); + conflux_log_set(LOG_WARN, circ->conflux, is_client); + } + circ->conflux->in_full_teardown = true; + } + } if (CONFLUX_NUM_LEGS(circ->conflux) > 0) { /* The last leg will free the streams but until then, we nullify to avoid @@ -2126,14 +2141,36 @@ conflux_log_set(int loglevel, const conflux_t *cfx, bool is_client) } } +/** + * Conflux needs a notification when tor_shutdown() begins, so that + * when circuits are freed, new legs are not launched. + * + * This needs a separate notification from conflux_pool_free_all(), + * because circuits must be freed before that function. + */ +void +conflux_notify_shutdown(void) +{ + shutting_down = true; +} + +#ifdef TOR_UNIT_TESTS +/** + * For unit tests: Clear the shutting down state so we resume building legs. + */ +void +conflux_clear_shutdown(void) +{ + shutting_down = false; +} +#endif + /** Free and clean up the conflux pool subsystem. This is called by the subsys * manager AFTER all circuits have been freed which implies that all objects in * the pools aren't referenced anymore. */ void conflux_pool_free_all(void) { - shutting_down = true; - digest256map_free(client_linked_pool, free_conflux_void_); digest256map_free(server_linked_pool, free_conflux_void_); digest256map_free(client_unlinked_pool, free_unlinked_void_); diff --git a/src/core/or/conflux_pool.h b/src/core/or/conflux_pool.h index afa4d9d058..eba726b03a 100644 --- a/src/core/or/conflux_pool.h +++ b/src/core/or/conflux_pool.h @@ -12,6 +12,7 @@ #include "core/or/or.h" void conflux_pool_init(void); +void conflux_notify_shutdown(void); void conflux_pool_free_all(void); origin_circuit_t *conflux_get_circ_for_conn(const entry_connection_t *conn, @@ -41,6 +42,7 @@ void conflux_log_set(int loglevel, const conflux_t *cfx, bool is_client); #ifdef TOR_UNIT_TESTS bool launch_new_set(int num_legs); +void conflux_clear_shutdown(void); digest256map_t *get_linked_pool(bool is_client); digest256map_t *get_unlinked_pool(bool is_client); extern uint8_t DEFAULT_CLIENT_UX; diff --git a/src/test/test_conflux_pool.c b/src/test/test_conflux_pool.c index fc30677377..05dc7b14ff 100644 --- a/src/test/test_conflux_pool.c +++ b/src/test/test_conflux_pool.c @@ -396,6 +396,7 @@ test_setup(void) static void test_clear_circs(void) { + conflux_notify_shutdown(); SMARTLIST_FOREACH(circ_pairs, circ_pair_t *, circ_pair, { tor_free(circ_pair); }); @@ -430,6 +431,9 @@ test_clear_circs(void) tor_assert(smartlist_len(mock_cell_delivery) == 0); (void)free_fake_origin_circuit; + + /* Clear shutdown flag so we can resume testing again. */ + conflux_clear_shutdown(); } static void