RFR: 8295125: os::signal should be os specific
David Holmes
dholmes at openjdk.org
Fri Oct 21 03:03:53 UTC 2022
On Thu, 20 Oct 2022 16:38:47 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> This was intended as a significant cleanup RFE but it turned out things were a lot messier than initially envisaged, so the resultant cleanup is quite limited.
>>
>> The OS API/namespace was simplified by removing three signal related functions that are not needed in shared code:
>> - signal
>> - user_handler
>> - raise
>> The first two were reworked and placed into PosixSignals and os::win32.
>> There was no need for a specific XX::raise API as os::raise simply called ::raise.
>>
>> On the Posix side we had the basic signal handler installation code in two places:
>>
>> 1. Inline in set_signal_handler and used by nearly all the VM signals
>> 2. In os::signal which was used by three different "clients"
>> - VM error handler for updating crash handler
>> - VM for installing SIG_BREAK handler
>> - JDK via JVM_RegisterSignal
>>
>> This was refactored so that we have:
>>
>> 1. PosixSignals::install_sigaction_signal_handler
>> - used by the VM only
>> - set_signal_handler
>> - VM error handler for updating crash handler
>> - VM for installing SIG_BREAK handler
>>
>> 2. PosixSignals::install_generic_signal_handler
>> - used by JDK via JVM_RegisterSignal
>>
>> The sigaction handlers were all consistently defined as per the posix definition:
>> void func(int signo, siginfo_t *info, void *context);
>>
>>
>> On the Windows side we just fixed the incorrect definition of UserHandler and made some consistency changes to use the right signal handler type where possible.
>>
>> Thanks.
>
> src/hotspot/os/posix/signals_posix.cpp line 877:
>
>> 875: // - sigAct: the new struct sigaction to be filled in and used
>> 876: // for this signal. The caller must provide this as it
>> 877: // may need to be stored/accessed by that caller.
>
> Maybe I missed something, but I didn't find any places where the caller needs sigAct.
It is in `set_signal_handlers`:
struct sigaction sigAct;
int ret = PosixSignals::install_sigaction_signal_handler(&sigAct, &oldAct,
sig, javaSignalHandler);
assert(ret == 0, "check");
// Save handler setup for possible later checking
vm_handlers.set(sig, &sigAct);
and is the reason I pass it in.
-------------
PR: https://git.openjdk.org/jdk/pull/10780
More information about the hotspot-runtime-dev
mailing list