RFR: JDK-8255711: Fix and unify hotspot signal handlers

[David Holmes]

On 4/11/2020 4:39 pm, Thomas Stüfe wrote:
>          > 6) At the end of every platform header, before calling into
>         fatal error handling, we unblock the signal:
>          >
>          >>   // unmask current signal
>          >>   sigset_t newset;
>          >>   sigemptyset(&newset);
>          >>   sigaddset(&newset, sig);
>          >>   sigprocmask(SIG_UNBLOCK, &newset, NULL);
>          >>
>          >
>          > - Use of `sigprocmask()` is UB in a multithreaded program.
>          > - but then, this section is unnecessary anyway, since
>         [https://bugs.openjdk.java.net/browse/JDK-8252533](JDK-8252533)
>         <https://bugs.openjdk.java.net/browse/JDK-8252533%5D(JDK-8252533)>
>         we unmask error signals at the start of the signal handler.
> 
>         But is this guaranteed to be one of the error signals? What if the
>         application calls our handler for some other signal? I guess
>         that is
>         their problem.
> 
> 
>     Good point, but:
>     - we only need to unblock error signals. There is no reasonable need
>     to unblock other signals in fatal error handling (to the contrary,
>     the rest should be kept blocked to not interfere with hs-err printing)
>     - we only install handlers for: SIGSEGV, SIGPIPE, SIGBUS, SIGILL,
>     SIGFPE, SIGTRAP, SIGXFSZ. Of those, we don't unblock SIGPIPE
>     and SIGXFSZ. But those are handled at the entrance of
>     javaSignalHandler_inner, so we should never enter fatal
>     error handling because of those.
>     But you raise an interesting point, I am not sure whether SIGXFSZ
>     can be deferred. What happens if we write a big core file and that
>     hits the file limit, but SIGXSFZ is blocked from delivery? Will we
>     just get terminated? Well, maybe we should get terminated. But this
>     is a question for another RFE.
> 
> 
> Missed part of your point here.
> 
> Calling this function from the outside is only allowed for a couple of 
> signals, see comment:
> 
> // This routine may recognize any of the following kinds of signals:
> //    SIGBUS, SIGSEGV, SIGILL, SIGFPE, SIGQUIT, SIGPIPE, SIGXFSZ, SIGUSR1.
> // It should be consulted by handlers for any of those signals.
> 
> Note that this list is not really correct, as it includes SIGUSR1 
> and SIGQUIT. None of which are handled by the hotspot signal handler. As 
> you wrote before, this mechanism is only to handle signals the hotspot 
> commandeers.

SIGUSR1 is probably a leftover from when we did use SIGUSR1 for something.

SIGQUIT is interesting because if the app is in charge of signals then 
it will install a SIGQUIT handler and we would not want the "VM" to do 
it's normal SIGQUIT handler. I have a big question mark over how the use 
of the signal handler thread can/should interact with 
AllowUserSignalHandlers.

> So I think JVM_handle_xx_signal() should test for the list of allowed 
> signals, and just return false right away in case sig is none of the 
> hotspot signals.

I don't think we should change existing behaviour here.

Thanks,
David

>