RFR: JDK-8252533: Signal handlers should run with synchronous error signals unblocked [v2]

[Thomas Stuefe]

On Thu, 29 Oct 2020 07:38:05 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> Hi Thomas,
>>> 
>>> I still feel that we should never explicitly block these error signals given it leads to undefined behaviour. So I'd like to see a change to os::signal at least in a future RFE.
>> 
>> Not a problem, that is reasonable. No need to make a new RFE. I will prepare a new version which excludes those signals from the sigaction sa_mask too, but leaves the unblocking in place. And I'll add the SR_handler. Since this will probably take some days to get tested again, I'll be happy if you review it after your vacation :)
>> 
>> Cheers, Thomas
>> 
>>> 
>>> For now I think all the signal handlers should explicitly unblock the error signals, which means the SR_handler also needs updating.
>>> 
>>> Some other minor changes requested below.
>>> 
>>> Thanks,
>>> David
>
>> Hi Thomas,
>> 
> .....
>> 
>> Aside: I also noticed the SR_sigset is unused, so unclear if we have a bug lurking in PosixSignals::SR_initialize. There is some commentary about the blocked/unblocked status of SR_signum that also seems wrong.
> 
> I find the code in SR_initialize strange and very probably broken. I created https://bugs.openjdk.java.net/browse/JDK-8255577 to keep track of this. Its on my name but if you'd like to take this on be my guest.

Changed in the new version:

- As David requested, we now exclude synchronous error signals from the signal masks used when registering a signal handler (sa_action.sa_mask). This means that unblocking them explicitly inside the signal handlers is only an additional safety measure now (the signal mask at the entrance of a signal handler is the result of an & operation between sa_action.sa_mask and whatever mask was in effect when the signal happened).

- As David requested, now the SR_handler unblocks error signals too, as well as the UserHandler.

- I wrote a big comment explaining what I do and why I do it.

-------------

PR: https://git.openjdk.java.net/jdk/pull/839