commit c21cfd28f43a969229ede02e20c6b554c1b88aae Author: teor teor2345@gmail.com Date: Fri May 26 16:16:37 2017 +1000

Make clients try fallbacks before authorities

Make clients wait for 6 seconds before trying to download their consensus from an authority.

Fixes bug 17750, bugfix on 0.2.8.1-alpha. --- changes/bug17750 | 4 ++++ src/or/directory.c | 31 +++++++++++++++++++++++++++---- src/or/directory.h | 7 ++++++- src/test/test_dir.c | 2 +- 4 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/changes/bug17750 b/changes/bug17750 new file mode 100644 index 0000000..eb77b77 --- /dev/null +++ b/changes/bug17750 @@ -0,0 +1,4 @@ + o Minor bugfixes (directory downloads): + - Make clients wait for 6 seconds before trying to download their + consensus from an authority. + Fixes bug 17750, bugfix on 0.2.8.1-alpha. diff --git a/src/or/directory.c b/src/or/directory.c index fce48c6..7d6a060 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3701,7 +3701,7 @@ connection_dir_finished_connecting(dir_connection_t *conn) * Helper function for download_status_increment_failure(), * download_status_reset(), and download_status_increment_attempt(). */ STATIC const smartlist_t * -find_dl_schedule(download_status_t *dls, const or_options_t *options) +find_dl_schedule(const download_status_t *dls, const or_options_t *options) { const int dir_server = dir_server_mode(options); const int multi_d = networkstatus_consensus_can_use_multiple_directories( @@ -3954,6 +3954,11 @@ download_status_increment_failure(download_status_t *dls, int status_code,

tor_assert(dls);

+ /* dls wasn't reset before it was used */ + if (dls->next_attempt_at == 0) { + download_status_reset(dls); + } + /* count the failure */ if (dls->n_download_failures < IMPOSSIBLE_TO_DOWNLOAD-1) { ++dls->n_download_failures; @@ -4006,6 +4011,11 @@ download_status_increment_attempt(download_status_t *dls, const char *item,

tor_assert(dls);

+ /* dls wasn't reset before it was used */ + if (dls->next_attempt_at == 0) { + download_status_reset(dls); + } + if (dls->increment_on == DL_SCHED_INCREMENT_FAILURE) { /* this schedule should retry on failure, and not launch any concurrent attempts */ @@ -4029,6 +4039,15 @@ download_status_increment_attempt(download_status_t *dls, const char *item, return dls->next_attempt_at; }

+static time_t +download_status_get_initial_delay_from_now(const download_status_t *dls) +{ + const smartlist_t *schedule = find_dl_schedule(dls, get_options()); + /* We use constant initial delays, even in exponential backoff + * schedules. */ + return time(NULL) + *(int *)smartlist_get(schedule, 0); +} + /** Reset <b>dls</b> so that it will be considered downloadable * immediately, and/or to show that we don't need it anymore. * @@ -4047,11 +4066,9 @@ download_status_reset(download_status_t *dls) || dls->n_download_attempts == IMPOSSIBLE_TO_DOWNLOAD) return; /* Don't reset this. */

- const smartlist_t *schedule = find_dl_schedule(dls, get_options()); - dls->n_download_failures = 0; dls->n_download_attempts = 0; - dls->next_attempt_at = time(NULL) + *(int *)smartlist_get(schedule, 0); + dls->next_attempt_at = download_status_get_initial_delay_from_now(dls); dls->last_backoff_position = 0; dls->last_delay_used = 0; /* Don't reset dls->want_authority or dls->increment_on */ @@ -4078,6 +4095,12 @@ download_status_get_n_attempts(const download_status_t *dls) time_t download_status_get_next_attempt_at(const download_status_t *dls) { + /* dls wasn't reset before it was used */ + if (dls->next_attempt_at == 0) { + /* so give the answer we would have given if it had been */ + return download_status_get_initial_delay_from_now(dls); + } + return dls->next_attempt_at; }

diff --git a/src/or/directory.h b/src/or/directory.h index 629b3ea..8ff16b3 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -114,6 +114,11 @@ static inline int download_status_is_ready(download_status_t *dls, time_t now, int max_failures) { + /* dls wasn't reset before it was used */ + if (dls->next_attempt_at == 0) { + download_status_reset(dls); + } + if (dls->backoff == DL_SCHED_DETERMINISTIC) { /* Deterministic schedules can hit an endpoint; exponential backoff * schedules just wait longer and longer. */ @@ -162,7 +167,7 @@ STATIC char* authdir_type_to_string(dirinfo_type_t auth); STATIC const char * dir_conn_purpose_to_string(int purpose); STATIC int should_use_directory_guards(const or_options_t *options); STATIC zlib_compression_level_t choose_compression_level(ssize_t n_bytes); -STATIC const smartlist_t *find_dl_schedule(download_status_t *dls, +STATIC const smartlist_t *find_dl_schedule(const download_status_t *dls, const or_options_t *options); STATIC void find_dl_min_and_max_delay(download_status_t *dls, const or_options_t *options, diff --git a/src/test/test_dir.c b/src/test/test_dir.c index cdc56ac..398398c 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -3806,7 +3806,7 @@ test_dir_download_status_increment(void *arg) tt_assert(next_at == TIME_MAX); tt_assert(download_status_get_n_failures(&dls_attempt) == 1); tt_assert(download_status_get_n_attempts(&dls_attempt) == 0); - tt_assert(mock_get_options_calls == 0); + tt_assert(mock_get_options_calls >= 1);

/* Check that an attempt reset works */ mock_get_options_calls = 0;