circuit: Make circuit_build_times_disabled take an or_options_t

That way, when we are parsing the options and LearnCircuitBuildTimeout is set
to 0, we don't assert trying to get the options list with get_options().

Fixes #21062

Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit is contained in:
David Goulet 2017-01-18 12:53:01 -05:00
parent fa00f2dce5
commit 0069d14753
6 changed files with 20 additions and 16 deletions

4
changes/bug21062 Normal file
View File

@ -0,0 +1,4 @@
o Minor bugfixex (config):
- Don't assert when trying to get the options list when
LearnCircuitBuildTimeout is set to 0 and we are actually currently
parsing the options. Fixes #21062; bugfix on tor-0.2.9.3-alpha.

View File

@ -1017,7 +1017,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ)
"Assuming clock jump. Purpose %d (%s)", timediff, "Assuming clock jump. Purpose %d (%s)", timediff,
circ->base_.purpose, circ->base_.purpose,
circuit_purpose_to_string(circ->base_.purpose)); circuit_purpose_to_string(circ->base_.purpose));
} else if (!circuit_build_times_disabled()) { } else if (!circuit_build_times_disabled(get_options())) {
/* Only count circuit times if the network is live */ /* Only count circuit times if the network is live */
if (circuit_build_times_network_check_live( if (circuit_build_times_network_check_live(
get_circuit_build_times())) { get_circuit_build_times())) {

View File

@ -105,12 +105,11 @@ get_circuit_build_timeout_ms(void)
* 6. If we are configured in Single Onion mode * 6. If we are configured in Single Onion mode
*/ */
int int
circuit_build_times_disabled(void) circuit_build_times_disabled(const or_options_t *options)
{ {
if (unit_tests) { if (unit_tests) {
return 0; return 0;
} else { } else {
const or_options_t *options = get_options();
int consensus_disabled = networkstatus_get_param(NULL, "cbtdisabled", int consensus_disabled = networkstatus_get_param(NULL, "cbtdisabled",
0, 0, 1); 0, 0, 1);
int config_disabled = !options->LearnCircuitBuildTimeout; int config_disabled = !options->LearnCircuitBuildTimeout;
@ -417,7 +416,7 @@ circuit_build_times_new_consensus_params(circuit_build_times_t *cbt,
* update if we aren't. * update if we aren't.
*/ */
if (!circuit_build_times_disabled()) { if (!circuit_build_times_disabled(get_options())) {
num = circuit_build_times_recent_circuit_count(ns); num = circuit_build_times_recent_circuit_count(ns);
if (num > 0) { if (num > 0) {
@ -493,14 +492,15 @@ static double
circuit_build_times_get_initial_timeout(void) circuit_build_times_get_initial_timeout(void)
{ {
double timeout; double timeout;
const or_options_t *options = get_options();
/* /*
* Check if we have LearnCircuitBuildTimeout, and if we don't, * Check if we have LearnCircuitBuildTimeout, and if we don't,
* always use CircuitBuildTimeout, no questions asked. * always use CircuitBuildTimeout, no questions asked.
*/ */
if (!unit_tests && get_options()->CircuitBuildTimeout) { if (!unit_tests && options->CircuitBuildTimeout) {
timeout = get_options()->CircuitBuildTimeout*1000; timeout = options->CircuitBuildTimeout*1000;
if (!circuit_build_times_disabled() && if (!circuit_build_times_disabled(options) &&
timeout < circuit_build_times_min_timeout()) { timeout < circuit_build_times_min_timeout()) {
log_warn(LD_CIRC, "Config CircuitBuildTimeout too low. Setting to %ds", log_warn(LD_CIRC, "Config CircuitBuildTimeout too low. Setting to %ds",
circuit_build_times_min_timeout()/1000); circuit_build_times_min_timeout()/1000);
@ -542,7 +542,7 @@ circuit_build_times_init(circuit_build_times_t *cbt)
* Check if we really are using adaptive timeouts, and don't keep * Check if we really are using adaptive timeouts, and don't keep
* track of this stuff if not. * track of this stuff if not.
*/ */
if (!circuit_build_times_disabled()) { if (!circuit_build_times_disabled(get_options())) {
cbt->liveness.num_recent_circs = cbt->liveness.num_recent_circs =
circuit_build_times_recent_circuit_count(NULL); circuit_build_times_recent_circuit_count(NULL);
cbt->liveness.timeouts_after_firsthop = cbt->liveness.timeouts_after_firsthop =
@ -906,7 +906,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt,
int err = 0; int err = 0;
circuit_build_times_init(cbt); circuit_build_times_init(cbt);
if (circuit_build_times_disabled()) { if (circuit_build_times_disabled(get_options())) {
return 0; return 0;
} }
@ -1507,7 +1507,7 @@ circuit_build_times_count_close(circuit_build_times_t *cbt,
int did_onehop, int did_onehop,
time_t start_time) time_t start_time)
{ {
if (circuit_build_times_disabled()) { if (circuit_build_times_disabled(get_options())) {
cbt->close_ms = cbt->timeout_ms cbt->close_ms = cbt->timeout_ms
= circuit_build_times_get_initial_timeout(); = circuit_build_times_get_initial_timeout();
return 0; return 0;
@ -1538,7 +1538,7 @@ void
circuit_build_times_count_timeout(circuit_build_times_t *cbt, circuit_build_times_count_timeout(circuit_build_times_t *cbt,
int did_onehop) int did_onehop)
{ {
if (circuit_build_times_disabled()) { if (circuit_build_times_disabled(get_options())) {
cbt->close_ms = cbt->timeout_ms cbt->close_ms = cbt->timeout_ms
= circuit_build_times_get_initial_timeout(); = circuit_build_times_get_initial_timeout();
return; return;
@ -1612,7 +1612,7 @@ circuit_build_times_set_timeout(circuit_build_times_t *cbt)
/* /*
* Just return if we aren't using adaptive timeouts * Just return if we aren't using adaptive timeouts
*/ */
if (circuit_build_times_disabled()) if (circuit_build_times_disabled(get_options()))
return; return;
if (!circuit_build_times_set_timeout_worker(cbt)) if (!circuit_build_times_set_timeout_worker(cbt))

View File

@ -17,7 +17,7 @@ circuit_build_times_t *get_circuit_build_times_mutable(void);
double get_circuit_build_close_time_ms(void); double get_circuit_build_close_time_ms(void);
double get_circuit_build_timeout_ms(void); double get_circuit_build_timeout_ms(void);
int circuit_build_times_disabled(void); int circuit_build_times_disabled(const or_options_t *options);
int circuit_build_times_enough_to_compute(const circuit_build_times_t *cbt); int circuit_build_times_enough_to_compute(const circuit_build_times_t *cbt);
void circuit_build_times_update_state(const circuit_build_times_t *cbt, void circuit_build_times_update_state(const circuit_build_times_t *cbt,
or_state_t *state); or_state_t *state);

View File

@ -1140,7 +1140,7 @@ needs_circuits_for_build(int num)
{ {
if (router_have_consensus_path() != CONSENSUS_PATH_UNKNOWN) { if (router_have_consensus_path() != CONSENSUS_PATH_UNKNOWN) {
if (num < CBT_MAX_UNUSED_OPEN_CIRCUITS && if (num < CBT_MAX_UNUSED_OPEN_CIRCUITS &&
!circuit_build_times_disabled() && !circuit_build_times_disabled(get_options()) &&
circuit_build_times_needs_circuits_now(get_circuit_build_times())) { circuit_build_times_needs_circuits_now(get_circuit_build_times())) {
return 1; return 1;
} }
@ -1400,7 +1400,7 @@ circuit_expire_old_circuits_clientside(void)
cutoff = now; cutoff = now;
last_expired_clientside_circuits = now.tv_sec; last_expired_clientside_circuits = now.tv_sec;
if (! circuit_build_times_disabled() && if (! circuit_build_times_disabled(get_options()) &&
circuit_build_times_needs_circuits(get_circuit_build_times())) { circuit_build_times_needs_circuits(get_circuit_build_times())) {
/* Circuits should be shorter lived if we need more of them /* Circuits should be shorter lived if we need more of them
* for learning a good build timeout */ * for learning a good build timeout */

View File

@ -3545,7 +3545,7 @@ options_validate(or_options_t *old_options, or_options_t *options,
int severity = LOG_NOTICE; int severity = LOG_NOTICE;
/* Be a little quieter if we've deliberately disabled /* Be a little quieter if we've deliberately disabled
* LearnCircuitBuildTimeout. */ * LearnCircuitBuildTimeout. */
if (circuit_build_times_disabled()) { if (circuit_build_times_disabled(options)) {
severity = LOG_INFO; severity = LOG_INFO;
} }
log_fn(severity, LD_CONFIG, "You disabled LearnCircuitBuildTimeout, but " log_fn(severity, LD_CONFIG, "You disabled LearnCircuitBuildTimeout, but "