veyron/runtime/google: fix potential race in shutdown_test
TestComplexServerDoubleSignal works by sending the child program two signals in
a row, and then verifying that the program exits with
signals.DoubleStopExitCode. The correctness of the test hinges upon ensuring
that the child program does not exit upon receiving its first signal. The
deferred function that removeCmdLoop returns is key for this, as it acts as a
sentinel to prevent the program from completing. There are two ways for
remoteCmdLoop to exit: (1) when the parent sends it the 'close' message over
stdin, and (2) when the parent closes stdin. The race occurs because of (2) --
the parent calls handle.Shutdown which closes the stdin of the child, thereby
triggering the remoteCmdLoop to return, which causes the deferred done drain to
proceeed. This may cause the chid program to run to completion before handling
of the second signal gets a chance to execute. Note, this was not an issue with
the old version of the test, since we never closed stdin explicitly. However,
with modules, there is no way to directly call Wait directly, and one must use
Shutdown (which has the side-effect of closing stdin).
The fix is to only close the done channel when the 'close' command is received
(i.e., case (1)) but not when stdin is closed (case (2)).
While at it, change the exit code for complex program upon receiving a double
signal to be the constant signals.DoubleStopExitCode rather than hardcoding 1
(in case we change the value of the constant down the road).
Change-Id: I8bc3fcc02a06b0e88f1a9d854fe8cab2141a6910
1 file changed
