Code review comment for lp:~jameinel/launchpad/lp-serve-child-hangup

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...
>> Would you rather just see it removed, since SIGALRM is killing the child
>> completely by default?
>
> My order of preference is
>
> 1- remove it and count on sigalrm actually killing it, including from the test
> 2- leave it and just add a comment it's not normally hit
> 3- leave it and also register a do-nothing handler to make sure it
> actually is hit
>
> I don't like 2 and 3 so much because there's some chance it will break
> if something else changes, and there may be other reasons we get an
> eintr earlier than we actually wanted to time out. (The latter is not
> very likely admittedly.) Also, all else being equal, less code is
> better.
>

If we want to test the function directly, the test suite has to have a
sigalrm handler, or it will kill the test run. So I left in a bit that
ensures we at least do a bit of cleanup, and raise an exception right
away. I suppose we could go one step further, and just not trap the
OSError and close the file descriptors. However, that causes the test
suite to leak file descriptors. Only 1 or 2, but it isn't very nice.

We do have a test that a spawned process dies with SIGALRM (because it
is part of the return code.)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk1yCfgACgkQJdeBCYSNAANRPwCgqHAvtGiUDweF6oYhfjul1N6x
jS8AoKVB83ldRCiCxsI2qPbgkwRpEUAL
=C2vK
-----END PGP SIGNATURE-----

« Back to merge proposal