August 2016 - tor-commits - lists.torproject.org

[tor/master] Compute thresholds for OOS from ConnLimit_ when setting options
by nickm＠torproject.org 25 Aug '16

25 Aug '16

commit 1c0c0022d8199a9b998a0c4e13a85a382abf7fb0 Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu Jun 30 10:26:13 2016 +0000 Compute thresholds for OOS from ConnLimit_ when setting options --- src/or/config.c | 26 ++++++++++++++++++++++++++ src/or/or.h | 4 ++++ 2 files changed, 30 insertions(+) diff --git a/src/or/config.c b/src/or/config.c index 3fab3b3..33c80e5 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1334,6 +1334,32 @@ options_act_reversible(const or_options_t *old_options, char **msg) connection_mark_for_close(conn); } }); + + if (set_conn_limit) { + /* + * If we adjusted the conn limit, recompute the OOS threshold too + * + * How many possible sockets to keep in reserve? If we have lots of + * possible sockets, keep this below a limit and set ConnLimit_high_thresh + * very close to ConnLimit_, but if ConnLimit_ is low, shrink it in + * proportion. + * + * Somewhat arbitrarily, set socks_in_reserve to 5% of ConnLimit_, but + * cap it at 64. + */ + int socks_in_reserve = options->ConnLimit_ / 20; + if (socks_in_reserve > 64) socks_in_reserve = 64; + + options->ConnLimit_high_thresh = options->ConnLimit_ - socks_in_reserve; + options->ConnLimit_low_thresh = (options->ConnLimit_ / 4) * 3; + log_info(LD_GENERAL, + "Recomputed OOS thresholds: ConnLimit %d, ConnLimit_ %d, " + "ConnLimit_high_thresh %d, ConnLimit_low_thresh %d", + options->ConnLimit, options->ConnLimit_, + options->ConnLimit_high_thresh, + options->ConnLimit_low_thresh); + } + goto done; rollback: diff --git a/src/or/or.h b/src/or/or.h index 1bac43c..262085e 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3699,6 +3699,10 @@ typedef struct { int ConnLimit; /**< Demanded minimum number of simultaneous connections. */ int ConnLimit_; /**< Maximum allowed number of simultaneous connections. */ + int ConnLimit_high_thresh; /**< start trying to lower socket usage if we + * have this many. */ + int ConnLimit_low_thresh; /**< try to get down to here after socket + * exhaustion. */ int RunAsDaemon; /**< If true, run in the background. (Unix only) */ int FascistFirewall; /**< Whether to prefer ORs reachable on open ports. */ smartlist_t *FirewallPorts; /**< Which ports our firewall allows

1 0

[tor/master] Implement kill_conn_list_for_oos()
by nickm＠torproject.org 25 Aug '16

25 Aug '16

commit e9464737af4b31e0aba29eb5581a5faf80647800 Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu Jun 30 14:13:42 2016 +0000 Implement kill_conn_list_for_oos() --- src/or/connection.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index fce63ad..e164fae 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -4519,9 +4519,20 @@ pick_oos_victims(int n) static void kill_conn_list_for_oos(smartlist_t *conns) { - (void)conns; + if (!conns) return; - /* TODO */ + SMARTLIST_FOREACH_BEGIN(conns, connection_t *, c) { + /* Make sure the channel layer gets told about orconns */ + if (c->type == CONN_TYPE_OR) { + connection_or_close_for_error(TO_OR_CONN(c), 1); + } else { + connection_mark_and_flush(c); + } + } SMARTLIST_FOREACH_END(c); + + log_notice(LD_NET, + "OOS handler marked and flushed %d connections", + smartlist_len(conns)); } /** Out-of-Sockets handler; n_socks is the current number of open

1 0

[tor/master] Unit test for pick_oos_victims()
by nickm＠torproject.org 25 Aug '16

25 Aug '16

commit d65f03091554e94ec1ceb51a607df19e9943c023 Author: Andrea Shepard <andrea(a)torproject.org> Date: Sat Jul 2 08:02:47 2016 +0000 Unit test for pick_oos_victims() --- src/or/connection_or.c | 4 +- src/or/connection_or.h | 2 +- src/or/main.c | 4 +- src/or/main.h | 2 +- src/test/test_oos.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 129 insertions(+), 7 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 1f0c4bd..72d8e13 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -394,8 +394,8 @@ connection_or_change_state(or_connection_t *conn, uint8_t state) * be an or_connection_t field, but it got moved to channel_t and we * shouldn't maintain two copies. */ -int -connection_or_get_num_circuits(or_connection_t *conn) +MOCK_IMPL(int, +connection_or_get_num_circuits, (or_connection_t *conn)) { tor_assert(conn); diff --git a/src/or/connection_or.h b/src/or/connection_or.h index e2ec47a..2e8c606 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -64,7 +64,7 @@ void connection_or_init_conn_from_address(or_connection_t *conn, int connection_or_client_learned_peer_id(or_connection_t *conn, const uint8_t *peer_id); time_t connection_or_client_used(or_connection_t *conn); -int connection_or_get_num_circuits(or_connection_t *conn); +MOCK_DECL(int, connection_or_get_num_circuits, (or_connection_t *conn)); void or_handshake_state_free(or_handshake_state_t *state); void or_handshake_state_record_cell(or_connection_t *conn, or_handshake_state_t *state, diff --git a/src/or/main.c b/src/or/main.c index 7f00cc3..18c0948 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -381,8 +381,8 @@ connection_in_array(connection_t *conn) /** Set <b>*array</b> to an array of all connections. <b>*array</b> must not * be modified. */ -smartlist_t * -get_connection_array(void) +MOCK_IMPL(smartlist_t *, +get_connection_array, (void)) { if (!connection_array) connection_array = smartlist_new(); diff --git a/src/or/main.h b/src/or/main.h index 45c3b3f..0220ae3 100644 --- a/src/or/main.h +++ b/src/or/main.h @@ -25,7 +25,7 @@ int connection_in_array(connection_t *conn); void add_connection_to_closeable_list(connection_t *conn); int connection_is_on_closeable_list(connection_t *conn); -smartlist_t *get_connection_array(void); +MOCK_DECL(smartlist_t *, get_connection_array, (void)); MOCK_DECL(uint64_t,get_bytes_read,(void)); MOCK_DECL(uint64_t,get_bytes_written,(void)); diff --git a/src/test/test_oos.c b/src/test/test_oos.c index 0f904f6..69a6fef 100644 --- a/src/test/test_oos.c +++ b/src/test/test_oos.c @@ -312,7 +312,7 @@ test_oos_kill_conn_list(void *arg) tt_int_op(cfe_calls, OP_EQ, 1); done: - + UNMOCK(connection_or_close_for_error); UNMOCK(connection_mark_for_close_internal_); @@ -323,10 +323,132 @@ test_oos_kill_conn_list(void *arg) return; } +static smartlist_t *conns_for_mock = NULL; + +static smartlist_t * +get_conns_mock(void) +{ + return conns_for_mock; +} + +/* + * For this mock, we pretend all conns have either zero or one circuits, + * depending on if this appears on the list of things to say have a circuit. + */ + +static smartlist_t *conns_with_circs = NULL; + +static int +get_num_circuits_mock(or_connection_t *conn) +{ + int circs = 0; + + tt_assert(conn != NULL); + + if (conns_with_circs && + smartlist_contains(conns_with_circs, TO_CONN(conn))) { + circs = 1; + } + + done: + return circs; +} + +static void +test_oos_pick_oos_victims(void *arg) +{ + (void)arg; + or_connection_t *ortmp; + dir_connection_t *dirtmp; + smartlist_t *picked; + + /* Set up mocks */ + conns_for_mock = smartlist_new(); + MOCK(get_connection_array, get_conns_mock); + conns_with_circs = smartlist_new(); + MOCK(connection_or_get_num_circuits, get_num_circuits_mock); + + /* Make some fake connections */ + ortmp = tor_malloc_zero(sizeof(*ortmp)); + ortmp->base_.magic = OR_CONNECTION_MAGIC; + ortmp->base_.type = CONN_TYPE_OR; + smartlist_add(conns_for_mock, TO_CONN(ortmp)); + /* We'll pretend this one has a circuit too */ + smartlist_add(conns_with_circs, TO_CONN(ortmp)); + /* Next one */ + ortmp = tor_malloc_zero(sizeof(*ortmp)); + ortmp->base_.magic = OR_CONNECTION_MAGIC; + ortmp->base_.type = CONN_TYPE_OR; + smartlist_add(conns_for_mock, TO_CONN(ortmp)); + /* Next one is moribund */ + ortmp = tor_malloc_zero(sizeof(*ortmp)); + ortmp->base_.magic = OR_CONNECTION_MAGIC; + ortmp->base_.type = CONN_TYPE_OR; + ortmp->base_.marked_for_close = 1; + smartlist_add(conns_for_mock, TO_CONN(ortmp)); + /* Last one isn't an orconn */ + dirtmp = tor_malloc_zero(sizeof(*dirtmp)); + dirtmp->base_.magic = DIR_CONNECTION_MAGIC; + dirtmp->base_.type = CONN_TYPE_DIR; + smartlist_add(conns_for_mock, TO_CONN(dirtmp)); + + /* Try picking one */ + picked = pick_oos_victims(1); + /* It should be the one with circuits */ + tt_assert(picked != NULL); + tt_int_op(smartlist_len(picked), OP_EQ, 1); + tt_assert(smartlist_contains(picked, smartlist_get(conns_for_mock, 0))); + smartlist_free(picked); + + /* Try picking none */ + picked = pick_oos_victims(0); + /* We should get an empty list */ + tt_assert(picked != NULL); + tt_int_op(smartlist_len(picked), OP_EQ, 0); + smartlist_free(picked); + + /* Try picking two */ + picked = pick_oos_victims(2); + /* We should get both active orconns */ + tt_assert(picked != NULL); + tt_int_op(smartlist_len(picked), OP_EQ, 2); + tt_assert(smartlist_contains(picked, smartlist_get(conns_for_mock, 0))); + tt_assert(smartlist_contains(picked, smartlist_get(conns_for_mock, 1))); + smartlist_free(picked); + + /* Try picking three - only two are eligible */ + picked = pick_oos_victims(3); + tt_int_op(smartlist_len(picked), OP_EQ, 2); + tt_assert(smartlist_contains(picked, smartlist_get(conns_for_mock, 0))); + tt_assert(smartlist_contains(picked, smartlist_get(conns_for_mock, 1))); + smartlist_free(picked); + + done: + + /* Free leftover stuff */ + if (conns_with_circs) { + smartlist_free(conns_with_circs); + conns_with_circs = NULL; + } + + UNMOCK(connection_or_get_num_circuits); + + if (conns_for_mock) { + SMARTLIST_FOREACH(conns_for_mock, connection_t *, c, tor_free(c)); + smartlist_free(conns_for_mock); + conns_for_mock = NULL; + } + + UNMOCK(get_connection_array); + + return; +} + struct testcase_t oos_tests[] = { { "connection_handle_oos", test_oos_connection_handle_oos, TT_FORK, NULL, NULL }, { "kill_conn_list", test_oos_kill_conn_list, TT_FORK, NULL, NULL }, + { "pick_oos_victims", test_oos_pick_oos_victims, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] Add connection_is_moribund() inline
by nickm＠torproject.org 25 Aug '16

25 Aug '16

commit 1a7709d40949b559a6f86e2bbcd1648eeeeed1af Author: Andrea Shepard <andrea(a)torproject.org> Date: Sat Aug 20 03:34:16 2016 +0000 Add connection_is_moribund() inline --- src/or/connection.c | 3 +-- src/or/connection.h | 14 ++++++++++++++ src/or/main.c | 5 +---- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index f38dc59..713a70c 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -4588,8 +4588,7 @@ pick_oos_victims, (int n)) ++(conn_counts_by_type[c->type]); /* Skip anything we would count as moribund */ - if (c->conn_array_index < 0 || - c->marked_for_close) { + if (connection_is_moribund(c)) { continue; } diff --git a/src/or/connection.h b/src/or/connection.h index 9bf063f..d25e002 100644 --- a/src/or/connection.h +++ b/src/or/connection.h @@ -247,6 +247,20 @@ void clock_skew_warning(const connection_t *conn, long apparent_skew, int trusted, log_domain_mask_t domain, const char *received, const char *source); +/** Check if a connection is on the way out so the OOS handler doesn't try + * to kill more than it needs. */ +static inline int +connection_is_moribund(connection_t *conn) +{ + if (conn != NULL && + (conn->conn_array_index < 0 || + conn->marked_for_close)) { + return 1; + } else { + return 0; + } +} + void connection_check_oos(int n_socks, int failed); #ifdef CONNECTION_PRIVATE diff --git a/src/or/main.c b/src/or/main.c index 9741f15..6ab94f5 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -662,10 +662,7 @@ connection_count_moribund, (void)) * runs next. */ SMARTLIST_FOREACH_BEGIN(closeable_connection_lst, connection_t *, conn) { - if (conn->conn_array_index < 0 || - conn->marked_for_close) { - ++moribund; - } + if (connection_is_moribund(conn)) ++moribund; } SMARTLIST_FOREACH_END(conn); return moribund;

1 0

[tor/master] Implement pick_oos_victims() except for sort step
by nickm＠torproject.org 25 Aug '16

25 Aug '16

commit d502efbd47abebea4f92ea55c4566f130c90ce0e Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu Jun 30 14:54:58 2016 +0000 Implement pick_oos_victims() except for sort step --- src/or/connection.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 79 insertions(+), 4 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index e164fae..18b8a7e 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -4506,13 +4506,88 @@ connection_reached_eof(connection_t *conn) static smartlist_t * pick_oos_victims(int n) { - smartlist_t *conns = NULL; + smartlist_t *eligible = NULL, *victims = NULL; + smartlist_t *conns; + int conn_counts_by_type[CONN_TYPE_MAX_ + 1], i; + + /* + * Big damn assumption (someone improve this someday!): + * + * Socket exhaustion normally happens on high-volume relays, and so + * most of the connections involved are orconns. We should pick victims + * by assembling a list of all orconns, and sorting them in order of + * how much 'damage' by some metric we'd be doing by dropping them. + * + * If we move on from orconns, we should probably think about incoming + * directory connections next, or exit connections. Things we should + * probably never kill are controller connections and listeners. + * + * This function will count how many connections of different types + * exist and log it for purposes of gathering data on typical OOS + * situations to guide future improvements. + */ + + /* First, get the connection array */ + conns = get_connection_array(); + /* + * Iterate it and pick out eligible connection types, and log some stats + * along the way. + */ + eligible = smartlist_new(); + memset(conn_counts_by_type, 0, sizeof(conn_counts_by_type)); + SMARTLIST_FOREACH_BEGIN(conns, connection_t *, c) { + /* Bump the counter */ + tor_assert(c->type <= CONN_TYPE_MAX_); + ++(conn_counts_by_type[c->type]); + + /* Skip anything we would count as moribund */ + if (c->conn_array_index < 0 || + c->marked_for_close) { + continue; + } + + switch (c->type) { + case CONN_TYPE_OR: + /* We've got an orconn, it's eligible to be OOSed */ + smartlist_add(eligible, c); + break; + default: + /* We don't know what to do with it, ignore it */ + break; + } + } SMARTLIST_FOREACH_END(c); - (void)n; + /* Log some stats */ + if (smartlist_len(conns) > 0) { + /* At least one counter must be non-zero */ + log_info(LD_NET, "Some stats on conn types seen during OOS follow"); + for (i = CONN_TYPE_MIN_; i <= CONN_TYPE_MAX_; ++i) { + /* Did we see any? */ + if (conn_counts_by_type[i] > 0) { + log_info(LD_NET, "%s: %d conns", + conn_type_to_string(i), + conn_counts_by_type[i]); + } + } + log_info(LD_NET, "Done with OOS conn type stats"); + } - /* TODO */ + /* Did we find more eligible targets than we want to kill? */ + if (smartlist_len(eligible) > n) { + /* TODO sort */ + /* Pick first n as victims */ + victims = smartlist_new(); + for (i = 0; i < n; ++i) { + smartlist_add(victims, smartlist_get(eligible, i)); + } + /* Free the original list */ + smartlist_free(eligible); + } else { + /* No, we can just call them all victims */ + victims = eligible; + } - return conns; + return victims; } /** Kill a list of connections for the OOS handler. */

1 0

[tor/master] Add DisableOOSCheck option, on by default until we have better heuristics
by nickm＠torproject.org 25 Aug '16

25 Aug '16

commit 31028af4404c06088ea1eb47309ce8c3154150f1 Author: Andrea Shepard <andrea(a)torproject.org> Date: Sat Aug 20 04:36:37 2016 +0000 Add DisableOOSCheck option, on by default until we have better heuristics --- src/or/config.c | 1 + src/or/connection.c | 5 +++++ src/or/or.h | 3 +++ 3 files changed, 9 insertions(+) diff --git a/src/or/config.c b/src/or/config.c index 4831bad..df85bc3 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -211,6 +211,7 @@ static config_var_t option_vars_[] = { V(CountPrivateBandwidth, BOOL, "0"), V(DataDirectory, FILENAME, NULL), V(DataDirectoryGroupReadable, BOOL, "0"), + V(DisableOOSCheck, BOOL, "1"), V(DisableNetwork, BOOL, "0"), V(DirAllowPrivateAddresses, BOOL, "0"), V(TestingAuthDirTimeToLearnReachability, INTERVAL, "30 minutes"), diff --git a/src/or/connection.c b/src/or/connection.c index 6b85bfb..daa11ee 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -4673,6 +4673,11 @@ connection_check_oos(int n_socks, int failed) int target_n_socks = 0, moribund_socks, socks_to_kill; smartlist_t *conns; + /* Early exit: is OOS checking disabled? */ + if (get_options()->DisableOOSCheck) { + return; + } + /* Sanity-check args */ tor_assert(n_socks >= 0); diff --git a/src/or/or.h b/src/or/or.h index 262085e..7eaeac8 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4458,6 +4458,9 @@ typedef struct { * participate in the protocol. If on (default), a flag is added to the * vote indicating participation. */ int AuthDirSharedRandomness; + + /** If 1, we skip all OOS checks. */ + int DisableOOSCheck; } or_options_t; /** Persistent state for an onion router, as saved to disk. */

1 0

[tor/master] Don't say OOS unless the ConnLimit thresholds have been set
by nickm＠torproject.org 25 Aug '16

25 Aug '16

commit 4f253d4c676caba979ddf4bc2668273700f279ae Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu Jun 30 10:58:17 2016 +0000 Don't say OOS unless the ConnLimit thresholds have been set --- src/or/connection.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 3939531..77ad567 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -4523,9 +4523,13 @@ connection_handle_oos(int n_socks, int failed) /* * Check if we're really handling an OOS condition, and if so decide how - * many sockets we want to get down to. + * many sockets we want to get down to. Be sure we check if the threshold + * is distinct from zero first; it's possible for this to be called a few + * times before we've finished reading the config. */ - if (n_socks > get_options()->ConnLimit_high_thresh) { + if (n_socks >= get_options()->ConnLimit_high_thresh && + get_options()->ConnLimit_high_thresh != 0 && + get_options()->ConnLimit_ != 0) { /* Try to get down to the low threshold */ target_n_socks = get_options()->ConnLimit_low_thresh; log_notice(LD_NET,

1 0

[tor/master] Stub out pick_oos_victims() and kill_conn_list_for_oos()
by nickm＠torproject.org 25 Aug '16

25 Aug '16

commit c76d45bdecb0ac5f11b578919e491957bee41d8f Author: Andrea Shepard <andrea(a)torproject.org> Date: Thu Jun 30 13:57:27 2016 +0000 Stub out pick_oos_victims() and kill_conn_list_for_oos() --- src/or/connection.c | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 6ef20b2..fce63ad 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -4500,6 +4500,30 @@ connection_reached_eof(connection_t *conn) } } +/** Pick n victim connections for the OOS handler and return them in a + * smartlist. + */ +static smartlist_t * +pick_oos_victims(int n) +{ + smartlist_t *conns = NULL; + + (void)n; + + /* TODO */ + + return conns; +} + +/** Kill a list of connections for the OOS handler. */ +static void +kill_conn_list_for_oos(smartlist_t *conns) +{ + (void)conns; + + /* TODO */ +} + /** Out-of-Sockets handler; n_socks is the current number of open * sockets, and failed is non-zero if a socket exhaustion related * error immediately preceded this call. This is where to do @@ -4509,6 +4533,7 @@ void connection_handle_oos(int n_socks, int failed) { int target_n_socks = 0, moribund_socks, socks_to_kill; + smartlist_t *conns; /* Sanity-check args */ tor_assert(n_socks >= 0); @@ -4563,10 +4588,16 @@ connection_handle_oos(int n_socks, int failed) if (moribund_socks < n_socks - target_n_socks) { socks_to_kill = n_socks - target_n_socks - moribund_socks; - /* TODO actually kill them */ - log_notice(LD_NET, - "OOS handler wants to kill %d sockets", - socks_to_kill); + + conns = pick_oos_victims(socks_to_kill); + if (conns) { + kill_conn_list_for_oos(conns); + log_notice(LD_NET, + "OOS handler killed %d conns", smartlist_len(conns)); + smartlist_free(conns); + } else { + log_notice(LD_NET, "OOS handler failed to pick any victim conns"); + } } else { log_notice(LD_NET, "Not killing any sockets for OOS because there are %d "

1 0

[tor/master] Unit test for kill_conn_list_for_oos()
by nickm＠torproject.org 25 Aug '16

25 Aug '16

commit e17083b4328490d8785c798eb4f41f653e6548d7 Author: Andrea Shepard <andrea(a)torproject.org> Date: Sat Jul 2 05:42:09 2016 +0000 Unit test for kill_conn_list_for_oos() --- src/or/connection.c | 6 ++-- src/or/connection.h | 4 +-- src/test/test_oos.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 844ab40..09b4733 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -754,9 +754,9 @@ connection_mark_for_close_(connection_t *conn, int line, const char *file) * For all other cases, use connection_mark_and_flush() instead, which * checks for or_connection_t properly, instead. See below. */ -void -connection_mark_for_close_internal_(connection_t *conn, - int line, const char *file) +MOCK_IMPL(void, +connection_mark_for_close_internal_, (connection_t *conn, + int line, const char *file)) { assert_connection_ok(conn,0); tor_assert(line); diff --git a/src/or/connection.h b/src/or/connection.h index 83ee769..e2f1764 100644 --- a/src/or/connection.h +++ b/src/or/connection.h @@ -34,8 +34,8 @@ void connection_about_to_close_connection(connection_t *conn); void connection_close_immediate(connection_t *conn); void connection_mark_for_close_(connection_t *conn, int line, const char *file); -void connection_mark_for_close_internal_(connection_t *conn, - int line, const char *file); +MOCK_DECL(void, connection_mark_for_close_internal_, + (connection_t *conn, int line, const char *file)); #define connection_mark_for_close(c) \ connection_mark_for_close_((c), __LINE__, SHORT_FILE__) diff --git a/src/test/test_oos.c b/src/test/test_oos.c index 48e66fd..0f904f6 100644 --- a/src/test/test_oos.c +++ b/src/test/test_oos.c @@ -8,6 +8,7 @@ #include "or.h" #include "config.h" #include "connection.h" +#include "connection_or.h" #include "main.h" #include "test.h" @@ -240,9 +241,92 @@ test_oos_connection_handle_oos(void *arg) return; } +static int cfe_calls = 0; + +static void +close_for_error_mock(or_connection_t *orconn, int flush) +{ + (void)flush; + + tt_assert(orconn != NULL); + ++cfe_calls; + + done: + return; +} + +static int mark_calls = 0; + +static void +mark_for_close_oos_mock(connection_t *conn, + int line, const char *file) +{ + (void)line; + (void)file; + + tt_assert(conn != NULL); + ++mark_calls; + + done: + return; +} + +static void +test_oos_kill_conn_list(void *arg) +{ + connection_t *c1, *c2; + or_connection_t *or_c1 = NULL; + dir_connection_t *dir_c2 = NULL; + smartlist_t *l = NULL; + (void)arg; + + /* Set up mocks */ + mark_calls = 0; + MOCK(connection_mark_for_close_internal_, mark_for_close_oos_mock); + cfe_calls = 0; + MOCK(connection_or_close_for_error, close_for_error_mock); + + /* Make fake conns */ + or_c1 = tor_malloc_zero(sizeof(*or_c1)); + or_c1->base_.magic = OR_CONNECTION_MAGIC; + or_c1->base_.type = CONN_TYPE_OR; + c1 = TO_CONN(or_c1); + dir_c2 = tor_malloc_zero(sizeof(*dir_c2)); + dir_c2->base_.magic = DIR_CONNECTION_MAGIC; + dir_c2->base_.type = CONN_TYPE_DIR; + c2 = TO_CONN(dir_c2); + + tt_assert(c1 != NULL); + tt_assert(c2 != NULL); + + /* Make list */ + l = smartlist_new(); + smartlist_add(l, c1); + smartlist_add(l, c2); + + /* Run kill_conn_list_for_oos() */ + kill_conn_list_for_oos(l); + + /* Check call counters */ + tt_int_op(mark_calls, OP_EQ, 1); + tt_int_op(cfe_calls, OP_EQ, 1); + + done: + + UNMOCK(connection_or_close_for_error); + UNMOCK(connection_mark_for_close_internal_); + + if (l) smartlist_free(l); + tor_free(or_c1); + tor_free(dir_c2); + + return; +} + struct testcase_t oos_tests[] = { { "connection_handle_oos", test_oos_connection_handle_oos, TT_FORK, NULL, NULL }, + { "kill_conn_list", test_oos_kill_conn_list, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] Use SMARTLIST_FOREACH in connection_count_moribund() per code review
by nickm＠torproject.org 25 Aug '16

25 Aug '16

commit a403230fe38d396189a32bc525aa596444d88b61 Author: Andrea Shepard <andrea(a)torproject.org> Date: Sat Aug 20 03:12:58 2016 +0000 Use SMARTLIST_FOREACH in connection_count_moribund() per code review --- src/or/main.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 18c0948..9741f15 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -655,20 +655,18 @@ close_closeable_connections(void) MOCK_IMPL(int, connection_count_moribund, (void)) { - int i, moribund = 0; - connection_t *conn; + int moribund = 0; /* * Count things we'll try to kill when close_closeable_connections() * runs next. */ - for (i = 0; i < smartlist_len(closeable_connection_lst); ++i) { - conn = smartlist_get(closeable_connection_lst, i); + SMARTLIST_FOREACH_BEGIN(closeable_connection_lst, connection_t *, conn) { if (conn->conn_array_index < 0 || conn->marked_for_close) { ++moribund; } - } + } SMARTLIST_FOREACH_END(conn); return moribund; }

1 0