commit 2212530bf59acb95ca9bb0278e51306e847105b7 Author: teor (Tim Wilson-Brown) <teor2345@gmail.com> Date: Mon Dec 7 18:07:44 2015 +1100 Prop210: Close excess connections once a consensus is downloading Once tor is downloading a usable consensus, any other connection attempts are not needed. Choose a connection to keep, favouring: * fallback directories over authorities, * connections initiated earlier over later connections Close all other connections downloading a consensus. --- doc/tor.1.txt | 7 +- src/or/directory.c | 218 +++++++++++++++++++++++++++++++++++++++++++- src/or/directory.h | 4 + src/or/main.c | 5 + src/or/networkstatus.c | 34 +++++++ src/or/networkstatus.h | 1 + src/test/test_config.c | 99 ++++++++++++++++++-- src/test/test_connection.c | 35 ++++++- 8 files changed, 388 insertions(+), 15 deletions(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 77e4c4e..2d95a54 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -360,8 +360,11 @@ GENERAL OPTIONS [[FallbackDir]] **FallbackDir** __address__:__port__ orport=__port__ id=__fingerprint__ [weight=__num__]:: When we're unable to connect to any directory cache for directory info - (usually because we don't know about any yet) we try a FallbackDir. - By default, the directory authorities are also FallbackDirs. + (usually because we don't know about any yet) we try a directory authority. + Clients also simultaneously try a FallbackDir, to avoid hangs on client + startup if a directory authority is down. Clients retry FallbackDirs more + often than directory authorities, to reduce the load on the directory + authorities. [[DirAuthority]] **DirAuthority** [__nickname__] [**flags**] __address__:__port__ __fingerprint__:: Use a nonstandard authoritative directory server at the provided address diff --git a/src/or/directory.c b/src/or/directory.c index 0d2a8b2..63bbdaf 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -961,6 +961,12 @@ directory_initiate_command_rend(const tor_addr_t *_addr, return; } + /* ensure we don't make excess connections when we're already downloading + * a consensus during bootstrap */ + if (connection_dir_avoid_extra_connection_for_purpose(dir_purpose)) { + return; + } + conn = dir_connection_new(tor_addr_family(&addr)); /* set up conn so it's got all the data we need to remember */ @@ -1001,6 +1007,9 @@ directory_initiate_command_rend(const tor_addr_t *_addr, conn->base_.state = DIR_CONN_STATE_CLIENT_SENDING; /* fall through */ case 0: + if (connection_dir_close_consensus_conn_if_extra(conn)) { + return; + } /* queue the command on the outbuf */ directory_send_command(conn, dir_purpose, 1, resource, payload, payload_len, @@ -1044,6 +1053,9 @@ directory_initiate_command_rend(const tor_addr_t *_addr, connection_mark_for_close(TO_CONN(conn)); return; } + if (connection_dir_close_consensus_conn_if_extra(conn)) { + return; + } conn->base_.state = DIR_CONN_STATE_CLIENT_SENDING; /* queue the command on the outbuf */ directory_send_command(conn, dir_purpose, 0, resource, @@ -3426,8 +3438,205 @@ connection_dir_finished_flushing(dir_connection_t *conn) return 0; } +/* A helper function for connection_dir_close_consensus_conn_if_extra() + * and connection_dir_close_extra_consensus_conns() that returns 0 if + * we can't have, or don't want to close, excess consensus connections. */ +int +connection_dir_would_close_consensus_conn_helper(void) +{ + const or_options_t *options = get_options(); + + /* we're only interested in closing excess connections if we could + * have created any in the first place */ + if (!networkstatus_consensus_can_use_multiple_directories(options)) { + return 0; + } + + /* We want to close excess connections downloading a consensus. + * If there aren't any excess, we don't have anything to close. */ + if (!networkstatus_consensus_has_excess_connections()) { + return 0; + } + + /* If we have excess connections, but none of them are downloading a + * consensus, and we are still bootstrapping (that is, we have no usable + * consensus), we don't want to close any until one starts downloading. */ + if (!networkstatus_consensus_is_downloading_usable_flavor() + && networkstatus_consensus_is_boostrapping(time(NULL))) { + return 0; + } + + /* If we have just stopped bootstrapping (that is, just parsed a consensus), + * we might still have some excess connections hanging around. So we still + * have to check if we want to close any, even if we've stopped + * bootstrapping. */ + return 1; +} + +/* Check if we would close excess consensus connections. If we would, any + * new consensus connection would become excess immediately, so return 1. + * Otherwise, return 0. */ +int +connection_dir_avoid_extra_connection_for_purpose(unsigned int purpose) +{ + const or_options_t *options = get_options(); + + /* We're not interested in connections that aren't fetching a consensus. */ + if (purpose != DIR_PURPOSE_FETCH_CONSENSUS) { + return 0; + } + + /* we're only interested in avoiding excess connections if we could + * have created any in the first place */ + if (!networkstatus_consensus_can_use_multiple_directories(options)) { + return 0; + } + + /* If there are connections downloading a consensus, and we are still + * bootstrapping (that is, we have no usable consensus), we can be sure that + * any further connections would be excess. */ + if (networkstatus_consensus_is_downloading_usable_flavor() + && networkstatus_consensus_is_boostrapping(time(NULL))) { + return 1; + } + + return 0; +} + +/* Check if we have excess consensus download connection attempts, and close + * conn: + * - if we don't have a consensus, and we're downloading a consensus, and conn + * is not downloading a consensus yet, close it; + * - if we do have a consensus, conn is excess, close it. */ +int +connection_dir_close_consensus_conn_if_extra(dir_connection_t *conn) +{ + tor_assert(conn); + tor_assert(conn->base_.type == CONN_TYPE_DIR); + + /* We're not interested in connections that aren't fetching a consensus. */ + if (conn->base_.purpose != DIR_PURPOSE_FETCH_CONSENSUS) { + return 0; + } + + /* The connection has already been closed */ + if (conn->base_.marked_for_close) { + return 0; + } + + if (!connection_dir_would_close_consensus_conn_helper()) { + return 0; + } + + const int we_are_bootstrapping = networkstatus_consensus_is_boostrapping( + time(NULL)); + + /* We don't want to check other connections to see if they are downloading, + * as this is prone to race-conditions. So leave it for + * connection_dir_consider_close_extra_consensus_conns() to clean up. + * + * But if conn has just started connecting, or we have a consensus already, + * we can be sure it's not needed any more. */ + if (!we_are_bootstrapping + || conn->base_.state == DIR_CONN_STATE_CONNECTING) { + connection_close_immediate(&conn->base_); + connection_mark_for_close(&conn->base_); + return -1; + } + + return 0; +} + +/* Check if we have excess consensus download connection attempts, and close + * them: + * - if we don't have a consensus, and we're downloading a consensus, keep an + * earlier connection, or a connection to a fallback directory, and close + * all other connections; + * - if we do have a consensus, close all connections: they are all excess. */ +void +connection_dir_close_extra_consensus_conns(void) +{ + if (!connection_dir_would_close_consensus_conn_helper()) { + return; + } + + int we_are_bootstrapping = networkstatus_consensus_is_boostrapping( + time(NULL)); + + const char *usable_resource = networkstatus_get_flavor_name( + usable_consensus_flavor()); + smartlist_t *consens_usable_conns = + connection_dir_list_by_purpose_and_resource( + DIR_PURPOSE_FETCH_CONSENSUS, + usable_resource); + + /* If we want to keep a connection that's downloading, find a connection to + * keep, favouring: + * - connections opened earlier (they are likely to have progressed further) + * - connections to fallbacks (to reduce the load on authorities) */ + dir_connection_t *kept_download_conn = NULL; + int kept_is_authority = 0; + if (we_are_bootstrapping) { + SMARTLIST_FOREACH_BEGIN(consens_usable_conns, + dir_connection_t *, d) { + tor_assert(d); + int d_is_authority = router_digest_is_trusted_dir(d->identity_digest); + /* keep the first connection that is past the connecting state, but + * prefer fallbacks. */ + if (d->base_.state != DIR_CONN_STATE_CONNECTING) { + if (!kept_download_conn || (kept_is_authority && !d_is_authority)) { + kept_download_conn = d; + kept_is_authority = d_is_authority; + /* we've found the earliest fallback, and want to keep it regardless + * of any other connections */ + if (!kept_is_authority) + break; + } + } + } SMARTLIST_FOREACH_END(d); + } + + SMARTLIST_FOREACH_BEGIN(consens_usable_conns, + dir_connection_t *, d) { + tor_assert(d); + /* don't close this connection if it's the one we want to keep */ + if (kept_download_conn && d == kept_download_conn) + continue; + /* mark all other connections for close */ + if (!d->base_.marked_for_close) { + connection_close_immediate(&d->base_); + connection_mark_for_close(&d->base_); + } + } SMARTLIST_FOREACH_END(d); + + smartlist_free(consens_usable_conns); + consens_usable_conns = NULL; + + /* make sure we've closed all excess connections */ + const int final_connecting_conn_count = + connection_dir_count_by_purpose_resource_and_state( + DIR_PURPOSE_FETCH_CONSENSUS, + usable_resource, + DIR_CONN_STATE_CONNECTING); + if (final_connecting_conn_count > 0) { + log_warn(LD_BUG, "Expected 0 consensus connections connecting after " + "cleanup, got %d.", final_connecting_conn_count); + } + const int expected_final_conn_count = (we_are_bootstrapping ? 1 : 0); + const int final_conn_count = + connection_dir_count_by_purpose_and_resource( + DIR_PURPOSE_FETCH_CONSENSUS, + usable_resource); + if (final_conn_count > expected_final_conn_count) { + log_warn(LD_BUG, "Expected %d consensus connections after cleanup, got " + "%d.", expected_final_conn_count, final_connecting_conn_count); + } +} + /** Connected handler for directory connections: begin sending data to the - * server */ + * server, and return 0, or, if the connection is an excess bootstrap + * connection, close all excess bootstrap connections. + * Only used when connections don't immediately connect. */ int connection_dir_finished_connecting(dir_connection_t *conn) { @@ -3438,7 +3647,12 @@ connection_dir_finished_connecting(dir_connection_t *conn) log_debug(LD_HTTP,"Dir connection to router %s:%u established.", conn->base_.address,conn->base_.port); - conn->base_.state = DIR_CONN_STATE_CLIENT_SENDING; /* start flushing conn */ + if (connection_dir_close_consensus_conn_if_extra(conn)) { + return -1; + } + + /* start flushing conn */ + conn->base_.state = DIR_CONN_STATE_CLIENT_SENDING; return 0; } diff --git a/src/or/directory.h b/src/or/directory.h index 4255868..37f9ab0 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -74,6 +74,9 @@ void directory_initiate_command(const tor_addr_t *addr, const char *resource, const char *payload, size_t payload_len, time_t if_modified_since); +int connection_dir_avoid_extra_connection_for_purpose(unsigned int purpose); +int connection_dir_close_consensus_conn_if_extra(dir_connection_t *conn); +void connection_dir_close_extra_consensus_conns(void); #define DSR_HEX (1<<0) #define DSR_BASE64 (1<<1) @@ -139,6 +142,7 @@ STATIC int directory_handle_command_get(dir_connection_t *conn, const char *headers, const char *req_body, size_t req_body_len); +int connection_dir_would_close_consensus_conn_helper(void); STATIC int download_status_schedule_get_delay(download_status_t *dls, const smartlist_t *schedule, time_t now); diff --git a/src/or/main.c b/src/or/main.c index 60957bd..455cba4 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1460,6 +1460,11 @@ run_scheduled_events(time_t now) dirvote_act(options, now); } + /* 2d. Cleanup excess consensus bootstrap connections every second. + * connection_dir_close_consensus_conn_if_extra() will close connections + * that are clearly excess, but this check is more thorough. */ + connection_dir_close_extra_consensus_conns(); + /* 3a. Every second, we examine pending circuits and prune the * ones which have been pending for more than a few seconds. * We do this before step 4, so it can try building more if diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 1d5b2f2..173c109 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1310,6 +1310,40 @@ networkstatus_consensus_can_use_extra_fallbacks(const or_options_t *options) > smartlist_len(router_get_trusted_dir_servers()))); } +/* Check if there is more than 1 consensus connection retrieving the usable + * consensus flavor. If so, return 1, if not, return 0. + * + * During normal operation, Tor only makes one consensus download + * connection. But clients can make multiple simultaneous consensus + * connections to improve bootstrap speed and reliability. + * + * If there is more than one connection, we must have connections left + * over from bootstrapping. However, some of the connections may have + * completed and been cleaned up, so it is not sufficient to check the + * return value of this function to see if a client could make multiple + * bootstrap connections. Use + * networkstatus_consensus_can_use_multiple_directories() + * and networkstatus_consensus_is_boostrapping(). */ +int +networkstatus_consensus_has_excess_connections(void) +{ + const char *usable_resource = networkstatus_get_flavor_name( + usable_consensus_flavor()); + const int consens_conn_usable_count = + connection_dir_count_by_purpose_and_resource( + DIR_PURPOSE_FETCH_CONSENSUS, + usable_resource); + /* The maximum number of connections we want downloading a usable consensus + * Always 1, whether bootstrapping or not. */ + const int max_expected_consens_conn_usable_count = 1; + + if (consens_conn_usable_count > max_expected_consens_conn_usable_count) { + return 1; + } + + return 0; +} + /* Is tor currently downloading a consensus of the usable flavor? */ int networkstatus_consensus_is_downloading_usable_flavor(void) diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h index d44022c..4cb33c3 100644 --- a/src/or/networkstatus.h +++ b/src/or/networkstatus.h @@ -75,6 +75,7 @@ int networkstatus_consensus_can_use_multiple_directories( const or_options_t *options); int networkstatus_consensus_can_use_extra_fallbacks( const or_options_t *options); +int networkstatus_consensus_has_excess_connections(void); int networkstatus_consensus_is_downloading_usable_flavor(void); #define NSSET_FROM_CACHE 1 diff --git a/src/test/test_config.c b/src/test/test_config.c index 28e9fa0..376dc1a 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -18,6 +18,7 @@ #include "entrynodes.h" #include "transports.h" #include "routerlist.h" +#include "networkstatus.h" static void test_config_addressmap(void *arg) @@ -1477,7 +1478,7 @@ test_config_adding_dir_servers(void *arg) (void)arg; /* allocate options */ - or_options_t *options = tor_malloc(sizeof(or_options_t)); + or_options_t *options = tor_malloc_zero(sizeof(or_options_t)); /* Allocate and populate configuration lines: * @@ -1486,8 +1487,7 @@ test_config_adding_dir_servers(void *arg) * Zeroing the structure has the same effect as initialising to: * { NULL, NULL, NULL, CONFIG_LINE_NORMAL, 0}; */ - config_line_t *test_dir_authority = tor_malloc(sizeof(config_line_t)); - memset(test_dir_authority, 0, sizeof(config_line_t)); + config_line_t *test_dir_authority = tor_malloc_zero(sizeof(config_line_t)); test_dir_authority->key = tor_strdup("DirAuthority"); test_dir_authority->value = tor_strdup( "D0 orport=9000 " @@ -1495,16 +1495,16 @@ test_config_adding_dir_servers(void *arg) "127.0.0.1:60090 0123 4567 8901 2345 6789 0123 4567 8901 2345 6789" ); - config_line_t *test_alt_bridge_authority = tor_malloc(sizeof(config_line_t)); - memset(test_alt_bridge_authority, 0, sizeof(config_line_t)); + config_line_t *test_alt_bridge_authority = tor_malloc_zero( + sizeof(config_line_t)); test_alt_bridge_authority->key = tor_strdup("AlternateBridgeAuthority"); test_alt_bridge_authority->value = tor_strdup( "B1 orport=9001 bridge " "127.0.0.1:60091 1123 4567 8901 2345 6789 0123 4567 8901 2345 6789" ); - config_line_t *test_alt_dir_authority = tor_malloc(sizeof(config_line_t)); - memset(test_alt_dir_authority, 0, sizeof(config_line_t)); + config_line_t *test_alt_dir_authority = tor_malloc_zero( + sizeof(config_line_t)); test_alt_dir_authority->key = tor_strdup("AlternateDirAuthority"); test_alt_dir_authority->value = tor_strdup( "A2 orport=9002 " @@ -1513,8 +1513,8 @@ test_config_adding_dir_servers(void *arg) ); /* Use the format specified in the manual page */ - config_line_t *test_fallback_directory = tor_malloc(sizeof(config_line_t)); - memset(test_fallback_directory, 0, sizeof(config_line_t)); + config_line_t *test_fallback_directory = tor_malloc_zero( + sizeof(config_line_t)); test_fallback_directory->key = tor_strdup("FallbackDir"); test_fallback_directory->value = tor_strdup( "127.0.0.1:60093 orport=9003 id=0323456789012345678901234567890123456789" @@ -1637,6 +1637,9 @@ test_config_adding_dir_servers(void *arg) /* we must have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 1); + /* we have more fallbacks than just the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1); + { /* fallback_dir_servers */ const smartlist_t *fallback_servers = router_get_fallback_dir_servers(); @@ -1669,7 +1672,10 @@ test_config_adding_dir_servers(void *arg) n_default_fallback_dir = (smartlist_len(fallback_servers) - n_default_alt_bridge_authority - n_default_alt_dir_authority); - /* If we have a negative count, something has gone really wrong */ + /* If we have a negative count, something has gone really wrong, + * or some authorities aren't being added as fallback directories. + * (networkstatus_consensus_can_use_extra_fallbacks depends on all + * authorities being fallback directories.) */ tt_assert(n_default_fallback_dir >= 0); } } @@ -1712,6 +1718,9 @@ test_config_adding_dir_servers(void *arg) /* we must not have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 0); + /* we have more fallbacks than just the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1); + { /* trusted_dir_servers */ const smartlist_t *dir_servers = router_get_trusted_dir_servers(); @@ -1849,6 +1858,9 @@ test_config_adding_dir_servers(void *arg) /* we must not have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 0); + /* we just have the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 0); + { /* trusted_dir_servers */ const smartlist_t *dir_servers = router_get_trusted_dir_servers(); @@ -1986,6 +1998,9 @@ test_config_adding_dir_servers(void *arg) /* we must not have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 0); + /* we have more fallbacks than just the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1); + { /* trusted_dir_servers */ const smartlist_t *dir_servers = router_get_trusted_dir_servers(); @@ -2124,6 +2139,9 @@ test_config_adding_dir_servers(void *arg) /* we must not have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 0); + /* we have more fallbacks than just the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 0); + { /* trusted_dir_servers */ const smartlist_t *dir_servers = router_get_trusted_dir_servers(); @@ -2272,6 +2290,9 @@ test_config_adding_dir_servers(void *arg) /* we must not have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 0); + /* we have more fallbacks than just the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1); + { /* trusted_dir_servers */ const smartlist_t *dir_servers = router_get_trusted_dir_servers(); @@ -2422,6 +2443,9 @@ test_config_adding_dir_servers(void *arg) /* we must have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 1); + /* we have more fallbacks than just the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1); + { /* trusted_dir_servers */ const smartlist_t *dir_servers = router_get_trusted_dir_servers(); @@ -2581,6 +2605,9 @@ test_config_adding_dir_servers(void *arg) /* we must not have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 0); + /* we have more fallbacks than just the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1); + { /* trusted_dir_servers */ const smartlist_t *dir_servers = router_get_trusted_dir_servers(); @@ -2734,6 +2761,9 @@ test_config_adding_dir_servers(void *arg) /* we must not have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 0); + /* we just have the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 0); + { /* trusted_dir_servers */ const smartlist_t *dir_servers = router_get_trusted_dir_servers(); @@ -2896,6 +2926,9 @@ test_config_adding_dir_servers(void *arg) /* we must not have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 0); + /* we have more fallbacks than just the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1); + { /* trusted_dir_servers */ const smartlist_t *dir_servers = router_get_trusted_dir_servers(); @@ -3055,6 +3088,9 @@ test_config_adding_dir_servers(void *arg) /* we must have added the default fallback dirs */ tt_assert(n_add_default_fallback_dir_servers_known_default == 1); + /* we have more fallbacks than just the authorities */ + tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1); + { /* trusted_dir_servers */ const smartlist_t *dir_servers = router_get_trusted_dir_servers(); @@ -3209,6 +3245,48 @@ test_config_adding_dir_servers(void *arg) UNMOCK(add_default_fallback_dir_servers); } +static void +test_config_use_multiple_directories(void *arg) +{ + (void)arg; + + or_options_t *options = tor_malloc_zero(sizeof(or_options_t)); + + /* Clients can use multiple directory mirrors for bootstrap */ + memset(options, 0, sizeof(or_options_t)); + options->ClientOnly = 1; + tt_assert(networkstatus_consensus_can_use_multiple_directories(options) + == 1); + + /* Bridge Clients can use multiple directory mirrors for bootstrap */ + memset(options, 0, sizeof(or_options_t)); + options->UseBridges = 1; + tt_assert(networkstatus_consensus_can_use_multiple_directories(options) + == 1); + + /* Bridge Relays (Bridges) must act like clients, and use multiple + * directory mirrors for bootstrap */ + memset(options, 0, sizeof(or_options_t)); + options->BridgeRelay = 1; + tt_assert(networkstatus_consensus_can_use_multiple_directories(options) + == 1); + + /* Clients set to FetchDirInfoEarly must fetch it from the authorities */ + memset(options, 0, sizeof(or_options_t)); + options->FetchDirInfoEarly = 1; + tt_assert(networkstatus_consensus_can_use_multiple_directories(options) + == 0); + + /* OR servers must fetch the consensus from the authorities */ + memset(options, 0, sizeof(or_options_t)); + options->ORPort_set = 1; + tt_assert(networkstatus_consensus_can_use_multiple_directories(options) + == 0); + + done: + tor_free(options); +} + #define CONFIG_TEST(name, flags) \ { #name, test_config_ ## name, flags, NULL, NULL } @@ -3222,6 +3300,7 @@ struct testcase_t config_tests[] = { CONFIG_TEST(check_or_create_data_subdir, TT_FORK), CONFIG_TEST(write_to_data_subdir, TT_FORK), CONFIG_TEST(fix_my_family, 0), + CONFIG_TEST(use_multiple_directories, 0), END_OF_TESTCASES }; diff --git a/src/test/test_connection.c b/src/test/test_connection.c index 2851387..1067b5f 100644 --- a/src/test/test_connection.c +++ b/src/test/test_connection.c @@ -644,43 +644,59 @@ test_conn_download_status(void *arg) /* no connections, no excess, not downloading */ tt_assert(networkstatus_consensus_has_excess_connections() == 0); tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 0); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 0); /* one connection, no excess, not downloading */ conn = test_conn_download_status_add_a_connection(); tt_assert(networkstatus_consensus_has_excess_connections() == 0); tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 0); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 0); /* one connection, no excess, but downloading */ conn->base_.state = TEST_CONN_DL_STATE; tt_assert(networkstatus_consensus_has_excess_connections() == 0); tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 1); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 1); conn->base_.state = TEST_CONN_STATE; /* two connections, excess, but not downloading */ conn2 = test_conn_download_status_add_a_connection(); tt_assert(networkstatus_consensus_has_excess_connections() == 1); tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 0); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 0); /* two connections, excess, downloading */ conn2->base_.state = TEST_CONN_DL_STATE; tt_assert(networkstatus_consensus_has_excess_connections() == 1); tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 1); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 1); conn2->base_.state = TEST_CONN_STATE; /* more connections, excess, but not downloading */ conn3 = test_conn_download_status_add_a_connection(); tt_assert(networkstatus_consensus_has_excess_connections() == 1); tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 0); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 0); /* more connections, excess, downloading */ conn3->base_.state = TEST_CONN_DL_STATE; tt_assert(networkstatus_consensus_has_excess_connections() == 1); tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 1); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 1); /* more connections, more downloading */ conn2->base_.state = TEST_CONN_DL_STATE; tt_assert(networkstatus_consensus_has_excess_connections() == 1); tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 1); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 1); /* now try closing the one that isn't downloading: * these tests won't work unless tor thinks it is bootstrapping */ @@ -689,22 +705,39 @@ test_conn_download_status(void *arg) tt_assert(connection_dir_count_by_purpose_and_resource( TEST_CONN_RSRC_PURPOSE, TEST_CONN_RSRC) == 3); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 1); tt_assert(connection_dir_close_consensus_conn_if_extra(conn) == -1); tt_assert(connection_dir_count_by_purpose_and_resource( TEST_CONN_RSRC_PURPOSE, TEST_CONN_RSRC) == 2); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 1); - /* now try closing one that is downloading - it stays open */ + /* now try closing one that is already closed - nothing happens */ tt_assert(connection_dir_close_consensus_conn_if_extra(conn) == 0); tt_assert(connection_dir_count_by_purpose_and_resource( TEST_CONN_RSRC_PURPOSE, TEST_CONN_RSRC) == 2); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 1); + + + /* now try closing one that is downloading - it stays open */ + tt_assert(connection_dir_close_consensus_conn_if_extra(conn2) == 0); + tt_assert(connection_dir_count_by_purpose_and_resource( + TEST_CONN_RSRC_PURPOSE, + TEST_CONN_RSRC) == 2); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 1); /* now try closing all excess connections */ connection_dir_close_extra_consensus_conns(); tt_assert(connection_dir_count_by_purpose_and_resource( TEST_CONN_RSRC_PURPOSE, TEST_CONN_RSRC) == 1); + tt_assert(connection_dir_avoid_extra_connection_for_purpose( + TEST_CONN_RSRC_PURPOSE) == 1); done: /* the teardown function removes all the connections */;