[tor-bugs] #26389 [Obfuscation/meek]: meek-client keep roundTripRetries on shutdown

[Tor Bug Tracker & Wiki]

#26389: meek-client keep roundTripRetries on shutdown
------------------------------+--------------------------
 Reporter:  cypherpunks       |          Owner:  dcf
     Type:  defect            |         Status:  reopened
 Priority:  Low               |      Milestone:
Component:  Obfuscation/meek  |        Version:
 Severity:  Normal            |     Resolution:
 Keywords:                    |  Actual Points:
Parent ID:                    |         Points:
 Reviewer:                    |        Sponsor:
------------------------------+--------------------------
Changes (by dcf):

 * priority:  Medium => Low


Comment:

 So, I think this is actually intentional behavior, though now that I look
 at it, it's conforming to an older version of pt-spec.txt and doing
 something that's no longer required.

 This is what the spec said from 2012 to 2014.
 https://gitweb.torproject.org/torspec.git/tree/pt-
 spec.txt?id=ffad9203dfda7848d780bf258dd141a2ea42e16f#n348
 > Proxies should respond to a single INT signal by closing their
 > listener ports and not accepting any new connections, but keeping
 > all connections open, then terminating when connections are all
 > closed.  Proxies should respond to a second INT signal by shutting
 > down cleanly.

 Keeping track of open connections was the whole purpose of `handlerChan`
 and the loop at the end of `main` that waits until all handlers are done.

 But the require to keep existing connections open
 [https://gitweb.torproject.org/torspec.git/commit/pt-
 spec.txt?id=4dcd7e94f17c072e771119ec90d7cbce4a4788a4 was removed] in 2014.
 This is what the current version says:
 https://gitweb.torproject.org/torspec.git/tree/pt-
 spec.txt?id=86480728d816474a0771a3b3aba5d223a32f0705#n620
 > PT proxies SHOULD handle OS specific mechanisms to gracefully
 > terminate (eg: Install a signal handler on 'SIGTERM' that
 > causes cleanup and a graceful shutdown if able).

 Now it only says that we have to do a graceful shutdown, which to me
 doesn't imply that we have to keep existing connections open. So we could
 just shut everything down, which seems to be what the ticket reporter
 expected anyway.

 The way I would recommend to do this would be to remove all the code
 having to do with `handlerChan`. When `main` returns, it will terminate
 the goroutine running `roundTripRetries`, I think. cypherpunks, do you
 want to test that? Otherwise, I don't think the current behavior is
 causing much of a problem, so I'll mark it as a low priority.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/26389#comment:5>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online