Opened 7 years ago

Closed 6 years ago

#10449 closed defect (fixed)

Tor launcher shuts down poorly when its Tor is a relay

Reported by: arma Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay
Cc: nickm, mcs, atagar, brade Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Edit your TBB's torrc file to include "ORPort 9001". Start TBB, let it bootstrap and produce a browser window.

Then close Tor Browser. Your stdout will say:

Dec 20 01:30:01.000 [notice] Owning controller connection has closed -- shutting down.
Dec 20 01:30:01.000 [notice] Interrupt: we have stopped accepting new connections, and will shut down in 30 seconds. Interrupt again to exit now.

and Tor Browser Bundle will exit:

Tor Browser exited cleanly.

Then thirty seconds later, we get

Dec 20 01:30:31.000 [notice] Clean shutdown finished. Exiting.

If you start your TBB again during this 30 second period, the old Tor process is still going, so your new Tor fails to start and things go bad.

What's going on here is due to Tor's ShutdownWaitLength default of 30 seconds.

Vidalia once solved this by popping up a window "you are a relay, do you want to close immediately or wait and close nicely"?

Maybe another answer is that Tor should close immediately, even if it's a relay, if it's closing because "Owning controller connection has closed".

Child Tickets

Change History (10)

comment:1 Changed 7 years ago by mcs

Cc: mcs added

comment:2 Changed 7 years ago by arma

Cc: atagar added
Component: Tor LauncherTor

Turning this into a Tor ticket -- I think the fix is that if Tor is shutting down because "Owning controller connection has closed", it should close right then even if it's a relay.

Damian, does this proposed behavior match how you think controllers would want Tor to behave?

comment:3 Changed 7 years ago by atagar

Yup, that's what I'd expect. Personally the 30 second wait is something I'd expect from a ctrl+c (ie, an interrupt signal). Other more non-interactive forms of killing I'd expect to kill tor right away.

comment:4 Changed 7 years ago by nickm

So, right now the behavior is defined as SIGINT in control-spec.txt. But that doesn't need to be the case; we could make it a fast exit instead.

Or we could add an argument to TAKEOWNERSHIP.

(Note that this is relevant only for crashing controllers: controllers that are doing a controlled shutdown can themselves decide how to turn off Tor.)

comment:5 in reply to:  4 Changed 7 years ago by arma

Replying to nickm:

(Note that this is relevant only for crashing controllers: controllers that are doing a controlled shutdown can themselves decide how to turn off Tor.)

Interesting point -- I assume we're having this conversation because tor launcher decided to leave it to tor how to shut down. Does that mean you recommend that Tor Launcher send Tor some other signal before closing the connection? This seems to be one of those cases where if we're going to recommend that every controller do some behavior, it's better to have it happen on the Tor side.

Do we know of any cases where, once you've set takeownership, for a Tor that's a relay, you'll want to let the Tor persist a while more even though the controller has vanished?

I would think that if the controller wants to do something fancy like a controlled shutdown of Tor, it shouldn't close its control port until it's done doing the fancy thing.

So to be clear, I think we'll be happier here just changing lost_owning_controller() so it calls tor_cleanup and exits.

comment:6 Changed 7 years ago by nickm

Milestone: Tor: 0.2.5.x-final
Status: newneeds_review

Trivial patch in branch "bug10449" in my tor repository. See also torspec patch in branch "bug10449" in my torspec repository.

This shouldn't take more than a minute to review.

comment:7 Changed 6 years ago by brade

Cc: brade added
Owner: changed from brade to nickm
Status: needs_reviewassigned

comment:8 Changed 6 years ago by brade

Status: assignedneeds_review

comment:9 Changed 6 years ago by arma

patch looks great to me

comment:10 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Thanks; Merged!

Note: See TracTickets for help on using tickets.