killing soffice.bin from oosplash (was: Re: [REVIEW][3-5] misc stuff)

On 19/04/12 13:35, Stephan Bergmann wrote:
> On 04/17/2012 03:15 PM, Michael Stahl wrote:
> > have oosplash forward SIGTERM to soffice.bin:
> >
> > http://cgit.freedesktop.org/libreoffice/core/commit/?id=946e7180be96178937e7be1b0bd7132902709a87
>
> The above is not a good idea.  At least with POSIX, there is no reliable 
> way for a parent process to forward to a child process a signal that the 
> parent receives.  There is always a window of time after fork(2) where 
> the forwarding will not yet happen, and there is always a window of time 
> after wait(2) where the forwarding will potentially send the signal to 
> an unrelated process.  (Yes, I know, the soffice script tries to do the 
> same, see 
> <http://compgroups.net/comp.unix.shell/sh-terminating-children-upon-sigterm/501864> 
> "sh: Terminating children upon SIGTERM" for a fruitless discussion.)
>
> The two processes need to communicate shutdown requests with a more 
> sophisticated protocol than trying to send a SIGTERM received by the 
> parent on to the child.

the soffice.bin really should be dead by the time the JUnit test process
exits, because otherwise the next time you try to run the test it won't
run (and even worse will hang forever currently...), because the old
soffice.bin is still listening on the pipe, so the new soffice.bin
cannot listen.

is there some POSIX interface that will kill all children of a process?

> That said:
>
> * g_pProcess should at least be volatile (it can't be sig_atomic_t).
>
> * New code should use sigaction(2), not signal(2).
>
> * The sigterm_handler returns normally, effectively ignoring the 
> SIGTERM.  Not sure if that is by design (as, if all goes well, 
> termination of the child will lead to termination of the parent, anyway) 
> or by accident.  (Though if it were by design, it would not take into 
> account that a SIGTERM sent before spawning the child does not lead to 
> timely termination.)

well the last time i read a book on how to write signal handlers is
clearly many years ago :)

the returning was intentional, so the oosplash could forward the exit
status of soffice.bin, but considering the race condition you mentioned
i guess hard-coding the exit status that soffice.bin would return is better.

i've tried to fix it a bit, please review:

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f92d22565d4d389b0c93d45a1995521431585694

> > these increase reliability of JUnit based test infrastructure:
> >
> > http://cgit.freedesktop.org/libreoffice/core/commit/?id=90f7a3ada68f309a9d3201183ef552e59f9558fb
> > http://cgit.freedesktop.org/libreoffice/core/commit/?id=228515e7783aecdb992258765554a530d6c831f3
>
> Not yet sure what to make of these two.  This is tricky territory, and a 
> number of potential deadlocks had deliberately been left in, on the 
> ground that (a) programmatically forcing termination of non-terminating 
> processes does not guarantee resource clean up (i.e., spawned 
> sub-processes can remain, see above), and (b) forcing termination 
> prevents debugging of the underlying deadlock.

these are not about deadlocks but to kill the office process in
situations where soffice.bin was not killed properly.

the first one was about a case where Desktop.terminate returned but
didn't terminate (because there was some unsaved document still open).

the second one was encountered when somebody removed an IDL type, and
the unoil.jar still contained the corresponding class file, which made
the Junit process' UNO runtime request the corresponding type over the
UNO bridge, but the other side didn't have it in its RDB so the Java UNO
bridge disposed itself, leading to an uncaught DisposedException here
and a still running soffice.bin process.

> It is a fallacy that a hung build can be killed automatically and the 
> machine be guaranteed to be in a clean state afterwards.

in case of a real deadlock these 2 commits shouldn't change anything.