RFR (XS) 8202014: Possible to receive signal before signal semaphore created

[David Holmes]

Hi Coleen,

On 18/05/2018 5:25 AM, coleen.phillimore at oracle.com wrote:
> 
> 
> On 5/16/18 2:04 AM, Thomas Stüfe wrote:
>> Hi,
>>
>> I agree with David. Signal handlers are installed in os::init_2(), but
>> the semaphore sometime later in the (imho misleadingly named) void
>> os::signal_init(). This looks like an initialization error.
>>
>> As a side note, can we make the "void signalHandler(int sig,
>> siginfo_t* info, void* uc)" functions in os_xxx.cpp static, since they
>> are only file local marshalling points?
> 
> Okay, I've moved the initialization of the signal semaphore to 
> os::init_2().   The signal handler for this signal_notify() case is set 
> up by JVM_RegisterSignal early in JDK initialization (initPhase1) which 
> is before os::signal_init() was run.  Now the semaphore is initialized 
> before that.

That seems fine.

Thanks,
David



> I also made signalHandler static in this change although not strictly 
> related.
> 
> http://cr.openjdk.java.net/~coleenp/8202014.02/webrev/index.html
> 
> Reran kill -INT script and mach5 hs_tiers 1 and 2. 
> test/hotspot/jtreg/runtime/signal tests and 
> test/jdk/sun/misc/SunMiscSignalTest.java.
> 
> Thanks,
> Coleen
> 
>>
>> Kind Regards, Thomas
>>
>>
>>
>>
>> On Wed, May 16, 2018 at 4:40 AM, David Holmes 
>> <david.holmes at oracle.com> wrote:
>>> Hi Coleen,
>>>
>>> On 16/05/2018 11:23 AM, coleen.phillimore at oracle.com wrote:
>>>> Summary: Don't assert for null semaphore because it can be null before
>>>> initialization is complete.
>>>
>>> Sorry I don't agree with this. Surely we should not install our signal
>>> handler until such time as everything is properly initialized? Or the
>>> handler itself should have some kind of initialization check to guide 
>>> how it
>>> responds.
>>>
>>> Thanks,
>>> David
>>>
>>>
>>>> Tested this with a shell script, thanks Robbin, and mach5 tier1,2 all
>>>> Oracle platforms.  AIX doesn't use the semaphore class so doesn't 
>>>> have this
>>>> bug.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8202014.01/webrev
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8202014
>>>>
>>>> Thanks,
>>>> Coleen
>