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

[Thomas Stuefe]

On Thu, 29 Oct 2020 02:43:40 GMT, David Holmes <dholmes 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

> src/hotspot/os/posix/vmError_posix.cpp line 105:
> 
>> 103: static void crash_handler(int sig, siginfo_t* info, void* ucVoid) {
>> 104: 
>> 105:   PosixSignals::unblock_error_signals();
> 
> Just to be clear this crash_handler is only installed for the error signals, so then original code was redundantly processing "sig" twice?

I'm unsure what you mean. Do you mean, it had been added twice to newset?

>  sigaddset(&newset, sig);  <<
>  // also unmask other synchronous signals
>  for (int i = 0; i < NUM_SIGNALS; i++) {
>    sigaddset(&newset, SIGNALS[i]);  <<
>  }
>  PosixSignals::unblock_thread_signal_mask(&newset);

That is true. I believe the original coding was from us too, so I feel responsible :)

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

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