Opened 4 years ago

Closed 4 months ago

#12062 closed defect (implemented)

Audit DisableNetwork, we_are_hibernating usage

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-client, technical-debt, quiet-mode, sponsor8-maybe, review-group-24
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor: Sponsor8-can

Description

I think a lot of our DisableNetwork checks should instead check net_is_disabled, since so much of what we're doing turning off when the network is disabled is also something we're trying to turn off when we're hibernating.

And probably some of our DisableNetwork checks should call should_delay_dir_fetches or something similar, if they're related to fetching non-bridge-descriptor directory stuff.

Possibly there should be a better designed hierarchy here.

Possibly, most of the fixes here should wait for 0.2.6, since this code is tricky.

Child Tickets

Change History (18)

comment:1 Changed 4 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.6.x-final

comment:2 Changed 4 years ago by nickm

Keywords: 026-triaged-1 added

comment:3 Changed 3 years ago by nickm

Milestone: Tor: 0.2.6.x-finalTor: 0.2.???

comment:4 Changed 16 months ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:5 Changed 14 months ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:6 Changed 9 months ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:7 Changed 9 months ago by nickm

Keywords: 026-triaged-1 removed

comment:8 Changed 9 months ago by nickm

Keywords: technical-debt quiet-mode sponsor8-maybe added
Severity: Normal

comment:9 Changed 8 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.2.x-final
Sponsor: Sponsor8-can

comment:10 Changed 5 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:11 Changed 5 months ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.3.x-final
Status: acceptedneeds_review

See my branch ticket12062. Delaying to 0.3.3 since this is tricky to test.

comment:12 Changed 5 months ago by nickm

(None of the bugs found here seem terrible, which is good)

comment:13 Changed 4 months ago by nickm

Keywords: review-group-24 added

review-group-24 is now open.

comment:14 Changed 4 months ago by dgoulet

Status: needs_reviewneeds_revision

I think the code added to should_delay_dir_fetches() is missing a "return 1".

The rest lgtm;

comment:15 Changed 4 months ago by nickm

added a fixup commit; squashed as ticket12062_squashed

comment:16 Changed 4 months ago by nickm

Status: needs_revisionmerge_ready

comment:17 Changed 4 months ago by nickm

merging to master. Thanks!

comment:18 Changed 4 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

(No backport planned -- this code is subtle and hard to test, and the bugs here are minor.)

Note: See TracTickets for help on using tickets.