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