meek-http-helper doesn't shutdown cleanly in 6.5a1

[arlolra]

I've got this process hanging around after I close TB 6.5a2,

/Applications/TorBrowser.app/Contents/MacOS/firefox --invisible -no-remote -profile /Applications/TorBrowser-Data/Tor/PluggableTransports/profile.meek-http-helper

Seems reproducible enough on this day.

Using meek-amazon. meek-azure no longer seems reachable.

[dcf]

I can reproduce this on 6.5a2 on linux.

While running:

     ├─firefox─┬─tor───meek-client-tor─┬─firefox─┬─firefox
     │         │                       │         ├─{BgHangManager}
     │         │                       │         ├─{Cache2 I/O}
     │         │                       │         ├─{Cert Verify}
     │         │                       │         ├─{Closing Service}
     │         │                       │         ├─{DNS Resolver #1}
     │         │                       │         ├─{Gecko_IOThread}
     │         │                       │         ├─{Hang Monitor}
     │         │                       │         ├─8*[{JS Helper}]
     │         │                       │         ├─{JS Watchdog}
     │         │                       │         ├─{Link Monitor}
     │         │                       │         ├─{SSL Cert #1}
     │         │                       │         ├─{SSL Cert #2}
     │         │                       │         ├─{Socket Thread}
     │         │                       │         ├─{StreamTrans #1}
     │         │                       │         ├─{Timer}
     │         │                       │         ├─{firefox}
     │         │                       │         └─{mozStorage #1}
     │         │                       ├─meek-client───10*[{meek-client}]
     │         │                       └─5*[{meek-client-tor}]

After quitting:

     ├─firefox─┬─firefox
     │         ├─{BgHangManager}
     │         ├─{Cache2 I/O}
     │         ├─{Cert Verify}
     │         ├─{Closing Service}
     │         ├─{DNS Resolver #1}
     │         ├─{Gecko_IOThread}
     │         ├─{Hang Monitor}
     │         ├─8*[{JS Helper}]
     │         ├─{JS Watchdog}
     │         ├─{Link Monitor}
     │         ├─{Socket Thread}
     │         ├─{Timer}
     │         ├─{firefox}
     │         └─{mozStorage #1}

[dcf]

Strangely enough, if I add -log meek-client-torbrowser.log after meek-client-torbrowser in Browser/TorBrowser/Data/Tor/torrc-defaults, then it kills the process cleanly. Adding the logging lines just causes a few log messages about killing the two child processes:

2016/09/14 19:31:27 sig terminated
2016/09/14 19:31:27 sending signal terminated to PID 4885
2016/09/14 19:31:27 killing PID 4885
2016/09/14 19:31:27 killing PID 4856

Maybe related to #15125.

[dcf]

The first release that has a problem is 6.5a1. I tested:

6.0.3 - ok
6.0.4 - ok
6.0.5 - ok

6.0a5 - ok
6.5a1 - bad
6.5a2 - bad

[dcf]

I've isolated the problem to the meek-client-torbrowser file. If you take a fresh installation of 6.0a5 (which does not have this bug), and copy Browser/TorBrowser/Tor/PluggableTransports/meek-client-torbrowser from 6.5a1, then the bug occurs.

The meek-client-torbrowser in 6.0a5 and 6.5a1 are built from slightly different sources: ​0.22-18371-2 versus 0.22-18371-3. That, however, doesn't seem to be the cause of the problem.

The actual problem seems to be changed Go version, ​from 1.4.2 to 1.6.2. If I recompile meek-client-torbrowser 0.22-18371-3 with go1.3.3 from Debian stable, then the error does not occur. If I compile it with go1.7.1 from Debian testing, the error does occur.

[dcf]

I tested compiling with various versions of Go and the earliest version that doesn't work is 1.6. 1.5.4 works, 1.6 doesn't.

Strangely enough, if I add -log meek-client-torbrowser.log after meek-client-torbrowser in Browser/TorBrowser/Data/Tor/torrc-defaults, then it kills the process cleanly.

Here's what's going wrong: once it receives a signal, meek-client-torbrowser writes to its log as it shuts down. By default the log lines go to stderr. But meek-client-torbrowser's stderr is the same as its parent process's (tor's) stderr. Once tor terminates, trying to write to the log results in a SIGPIPE that kills meek-client-torbrowser before it can terminate its subprocesses.

Why this only started happening with Go 1.6? See this ​excerpt from the release notes:

On Unix-like systems, when a write to os.Stdout or os.Stderr (more precisely, an os.File opened for file descriptor 1 or 2) fails due to a broken pipe error, the program will raise a SIGPIPE signal. By default this will cause the program to exit; this may be changed by calling the os/signal Notify function for syscall.SIGPIPE. A write to a broken pipe on a file descriptor other 1 or 2 will simply return syscall.EPIPE (possibly wrapped in os.PathError and/or os.SyscallError) to the caller. The old behavior of raising an uncatchable SIGPIPE signal after 10 consecutive writes to a broken pipe no longer occurs.

That's why logging to a file avoided the error: the output pipe was no longer being broken.

[dcf]

attachment:0001-Ignore-SIGPIPE-in-meek-client-torbrowser.patch​ works around the problem by ignoring SIGPIPE. I found I had to call signal.Notify with a dummy channel; calling signal.Ignore didn't work.

I think we're fine with ignoring logging errors. The log package ignores them anyway. I'm a bit worried that because Go's SIGPIPE rule affects both stdout and stderr, something might go wrong with the PT negotiation on stdout. goptlib doesn't report errors that may arise from its writes to stdout. But I couldn't think of a way it would cause a problem, and even if it does it should be rare.

arlolra, if you want to test this, you can just go build in the meek-client-torbrowser subdirectory, then copy meek-client-torbrowser into your bundle directory.

[yawning]

You can and should do better on Linux, than just ignoring SIGPIPE (which is necessary for OSX).

    cmd = exec.Command(absFirefoxPath, "--invisible", "-no-remote", "-profile", profilePath)
    cmd.SysProcAttr = &syscall.SysProcAttr{ Pdeathsig: syscall.SIGTERM}

[arlolra]

arlolra, if you want to test this, you can just go build in the meek-client-torbrowser subdirectory, then copy meek-client-torbrowser into your bundle directory.

Yup, seems to work.

[dcf]

Replying to arlolra:

arlolra, if you want to test this, you can just go build in the meek-client-torbrowser subdirectory, then copy meek-client-torbrowser into your bundle directory.

Yup, seems to work.

Thanks, merged in ​125b0ca1.

Replying to yawning:

You can and should do better on Linux, than just ignoring SIGPIPE (which is necessary for OSX).

cmd = exec.Command(absFirefoxPath, "--invisible", "-no-remote", "-profile", profilePath)
cmd.SysProcAttr = &syscall.SysProcAttr{ Pdeathsig: syscall.SIGTERM}

Good idea, did this in ​a3cc514d.

[dcf]

Ticket #20290 asks to include this fix in Tor Browser.