Opened 12 months ago

Closed 4 weeks ago

#20963 closed defect (fixed)

[notice] The Tor Directory Consensus has changed how many circuits we must track to detect network failures from 0 to 20.

Reported by: arma Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version: Tor: 0.2.8.9
Severity: Normal Keywords: 029-backport, 031-deferred-20170425
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Running Tor 0.2.8.9 under Yawning's sandboxed-tor-browser, I got this strange log message:

2016/12/13 15:48:04 tor: Dec 13 20:48:04.000 [notice] The Tor Directory Consensus has changed how many circuits we must track to detect network failures from 0 to 20.

I haven't seen it before. Also, my system ran out of disk space for a while before this notice came up.

A) I don't think the consensus changed its parameter here, so it's weird that my Tor thinks it did. 20 is the default:

or.h:#define CBT_DEFAULT_RECENT_CIRCUITS 20

So where did 0 come from?

B) If "A" remains a mystery, we should make sure there isn't some underlying bug where for example Tor starts ignoring things in the consensus after an event like

2016/12/13 13:48:04 tor: Dec 13 18:48:04.000 [warn] Error writing to "/home/amnesia/tor/data/cached-microdesc-consensus": No space left on device

Child Tickets

Change History (10)

comment:1 Changed 12 months ago by nickm

Keywords: 028-backport 029-backport added
Milestone: Tor: 0.3.0.x-final

comment:2 Changed 12 months ago by teor

Status: newneeds_information

I think this is as designed:

I bet you encountered condition 4 when you ran out of disk space:

/**
 * This function decides if CBT learning should be disabled. It returns
 * true if one or more of the following conditions are met:
 *
 *  1. If the cbtdisabled consensus parameter is set.
 *  2. If the torrc option LearnCircuitBuildTimeout is false.
 *  3. If we are a directory authority
 *  4. If we fail to write circuit build time history to our state file.
 *  5. If we are compiled or configured in Tor2web mode
 *  6. If we are configured in Single Onion mode
 */
int
circuit_build_times_disabled(void)

Which then led to:

  if (!circuit_build_times_disabled()) {
...
  } else {
    /*
     * Adaptive timeouts are disabled; this might be because of the
     * LearnCircuitBuildTimes config parameter, and hence permanent, or
     * the cbtdisabled consensus parameter, so it may be a new condition.
     * Treat it like getting num == 0 above and free the circuit history
     * if we have any.
     */

    circuit_build_times_free_timeouts(cbt);
  }

Which then led to:

/**
 * Free the saved timeouts, if the cbtdisabled consensus parameter got turned
 * on or something.
 */

void
circuit_build_times_free_timeouts(circuit_build_times_t *cbt)
{
  if (!cbt) return;

  if (cbt->liveness.timeouts_after_firsthop) {
    tor_free(cbt->liveness.timeouts_after_firsthop);
  }

  cbt->liveness.num_recent_circs = 0;
}

And then the next time you loaded a consensus, that 0 was used as the value.

So I can't see how we'd fix that, but we could make the error message better.

comment:3 Changed 10 months ago by nickm

Milestone: Tor: 0.3.0.x-finalTor: 0.3.1.x-final

comment:4 Changed 8 months ago by nickm

Keywords: 031-deferred-20170425 added
Milestone: Tor: 0.3.1.x-finalTor: 0.3.2.x-final

Triage: batch-defer unowned items of priority Medium or lower to 0.3.2.

comment:5 Changed 7 months ago by nickm

Keywords: 028-backport removed

comment:6 Changed 3 months ago by nickm

Any suggestions for what the error message should be?

comment:7 Changed 4 weeks ago by nickm

Owner: set to nickm
Status: needs_informationaccepted

I've tried to take a stab at this for bug20963_032; not sure it's really the right solution, or indeed that any solution is called for here. I say we either merge this, or just close the ticket as notabug.

comment:8 Changed 4 weeks ago by nickm

Status: acceptedneeds_review

comment:9 Changed 4 weeks ago by ahf

Status: needs_reviewmerge_ready

The patch looks good to me. Unsure if arma still wants to comment on what the error message should be?

comment:10 Changed 4 weeks ago by nickm

Resolution: fixed
Status: merge_readyclosed

Thanks for the review! I'm merging this to 0.3.2 and forward; I'm guessing Roger isn't going to get to this soon.

Note: See TracTickets for help on using tickets.