Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T18:35:13Zhttps://gitlab.torproject.org/legacy/trac/-/issues/15593Bring sanity to the tor side of the PT shutdown process.2020-06-13T18:35:13ZYawning AngelBring sanity to the tor side of the PT shutdown process.This is the final phase of my great PT shutdown process cleanup as a follow up to #15545.
Now that there's a portable mechanism to signal termination to PTs (close the stdin), we should change the PT shutdown process to allow graceful t...This is the final phase of my great PT shutdown process cleanup as a follow up to #15545.
Now that there's a portable mechanism to signal termination to PTs (close the stdin), we should change the PT shutdown process to allow graceful termination to look like this:
1. Close stdin (and on U*IX, send a SIGTERM, PT behavior here is equivalent).
2. Wait for a grace period (~1 sec?)
3. If the child still is not dead, send a SIGKILL/TerminateProcess(). (Failsafe)
This fixes #9330 in that, PTs that wish to trap a graceful shutdown on Windows have a way to do so, despite the final stage of the process killing the PT in the most violent way possible as a failsafe (realistically, PTs should exit shortly after step 1).Tor: unspecifiedYawning AngelYawning Angelhttps://gitlab.torproject.org/legacy/trac/-/issues/15545Document TOR_PT_EXIT_ON_STDIN_CLOSE in the pt-spec.2021-11-15T18:55:15ZYawning AngelDocument TOR_PT_EXIT_ON_STDIN_CLOSE in the pt-spec.This is the ticket for the documentation side of the `TOR_PT_EXIT_ON_STDIN_CLOSE` and associated behavior that was implemented as part of #15435.
`pt-spec.txt` needs to document that if the env var is set to `1`, then PTs should assume ...This is the ticket for the documentation side of the `TOR_PT_EXIT_ON_STDIN_CLOSE` and associated behavior that was implemented as part of #15435.
`pt-spec.txt` needs to document that if the env var is set to `1`, then PTs should assume that tor will keep stdin open, and to treat stdin being closed as the same as a `SIGTERM` (graceful shutdown immediately).Tor: 0.2.8.x-finalYawning AngelYawning Angelhttps://gitlab.torproject.org/legacy/trac/-/issues/15471Tor should prctl(PR_SET_PDEATHSIG, SIGTERM) background processes.2020-06-13T14:44:42ZYawning AngelTor should prctl(PR_SET_PDEATHSIG, SIGTERM) background processes.From the man page:
```
PR_SET_PDEATHSIG (since Linux 2.1.57)
Set the parent process death signal of the calling process to
arg2 (either a signal value in the range 1..maxsig, or 0 to
...From the man page:
```
PR_SET_PDEATHSIG (since Linux 2.1.57)
Set the parent process death signal of the calling process to
arg2 (either a signal value in the range 1..maxsig, or 0 to
clear). This is the signal that the calling process will get
when its parent dies. This value is cleared for the child of a
fork(2) and (since Linux 2.4.36 / 2.6.23) when executing a set-
user-ID or set-group-ID binary, or a binary that has associated
capabilities (see capabilities(7)). This value is preserved
across execve(2).
```
This will ensure at least on Linux that all background processes will get a `SIGTERM` if Tor dies, and can cleanup appropriately.
I don't think this behavior would be particularly shocking to PT developers (who should be handling SIGTERM already), so this probably doesn't even need a spec patch since "tor dying" invoking the normal termination signaling is appropriate.
The choice of `SIGTERM` over `SIGKILL` here is so that PTs have the option to trap it and terminate their own children as appropriate.
Pros:
* Easy to do.
* The kernel does all of the heavy lifting for us, as a catchall.
* Fixes certain nasty issues in unmodified pt code on the relevant platform automatically.
Cons:
* Non-portable (#15435 aka #10047 is a portable fix that requires pt modification).
* We have pts (obfs4proxy in particular) that can be ran with elevated capabilities.Tor: 0.2.7.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/15435Tor should not close stdin on child processes.2020-06-15T23:25:11ZYawning AngelTor should not close stdin on child processes.The lack of PTs having a reliable way to detect if tor is dead reared it's ugly head again in the form of #15434. There aren't that many good ways to detect if a parent has died, beyond checking if stdin is still open (#10047 says stdou...The lack of PTs having a reliable way to detect if tor is dead reared it's ugly head again in the form of #15434. There aren't that many good ways to detect if a parent has died, beyond checking if stdin is still open (#10047 says stdout, however stdin is better), but tor explicitly closes the child process's stdin.
Since a variant of this has been working well, I propose that we make it official like thus:
* Add a new enviornment variable of the form `TOR_PT_EXIT_ON_STDIN_CLOSE=1`, indicating that Tor will not close the stdin.
* PTs SHOULD terminate as if they have received a SIGTERM, on stdin being closed if the environment variable is set.
* All new tor versions will naturally set said env. var, and setup the pt plugin stdin correctly.
This does mean that Bridges will consume one extra file descriptor per `ServerTransportPlugin` line in the torrc, but I view this as acceptable given the fact that this will bring some much needed sanity to our cleanup process.Tor: 0.2.7.x-final