Handling of USR2 signal in JVM on Linux

[Andrey Turbanov]

>not C++ undefined behavior, nothing to do with C++

>From 'signals_posix.cpp':

Thread* thread = Thread::current_or_null_safe();
assert(thread != NULL, "Missing current thread in SR_handler");

if (thread->has_terminated()) {
  return;
}

In release build there will be no assert and 'thread' will be 'NULL'.
This is C++ code and in C++ dereferencing of NULL pointers is
undefined behaviour.
Do I miss something?




Andrey Turbanov

ср, 15 июн. 2022 г. в 13:45, Thomas Stüfe <thomas.stuefe at gmail.com>:
>
> More specifically, I propose to gracefully ignore SIGUSR2 in release builds if the receiving thread is not a java thread, but to retain the assert in debug builds.
>
> We could make it more involved by checking the sender pid and accepting only signals from hotspot threads themselves, but I do not think this complexity is necessary.
>
> Cheers, Thomas
>
> On Wed, Jun 15, 2022 at 12:26 PM Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>>
>> SIGUSR2 is used by the hotspot, internally, to implement suspend/resume. It gets sent by hotspot via pthread_kill() to targeted threads to suspend them. In that case it is known that the receiving thread is a valid java thread and therefore the assert makes sense.
>>
>> However, as you describe SIGUSR2 can also be sent from outside via kill(2). In that case the receiving thread is arbitrarily chosen by the kernel. It is not necessarily a valid java thread. In that case the VM will crash (release) or assert (debug).
>>
>> I tend to think this is an error too. Or at least in grey area. Since this is very easy to fix in the hotspot, I'd suggest we do this.
>>
>> If nobody objects, I can file an issue and prepare the patch.
>>
>> Cheers, Thomas
>>
>> (P.s. not C++ undefined behavior, nothing to do with C++ :-)
>>
>> On Wed, Jun 15, 2022 at 12:11 PM Andrey Turbanov <turbanoff at gmail.com> wrote:
>>>
>>> I mean, isn't JVM supposed to be safe? :)
>>> Receiving this signal _could_ happen in a real deployment. And now, as
>>> I can see, we have C++ undefined behaviour in release builds in this
>>> case. Can we consider this as a bug?
>>>
>>> Andrey Turbanov
>>>
>>> вт, 14 июн. 2022 г. в 14:46, Alan Bateman <Alan.Bateman at oracle.com>:
>>> >
>>> > On 14/06/2022 10:44, Andrey Turbanov wrote:
>>> > > Hello.
>>> > > During investigation of signal handling in JVM (for
>>> > > https://github.com/openjdk/jdk/pull/9100#discussion_r894992558 )
>>> > > I found out that sending USR2 crashes my JDK. (Linux fastdebug x64)
>>> > >
>>> > > kill -USR2 1346792
>>> > >
>>> > > # assert(thread != __null) failed: Missing current thread in SR_handler
>>> > > # Internal Error
>>> > > (/home/turbanoff/Projects/official_jdk/src/hotspot/os/posix/signals_posix.cpp:1600),
>>> > > pid=1346792, tid=1346792
>>> > >
>>> > > Full hs_err_pid1346792.log:
>>> > > https://gist.github.com/turbanoff/2099327ea13357a90df43a2d6b0e2e6a
>>> > >
>>> > >
>>> > > Is it known/expected behaviour?
>>> > > I found some description there
>>> > > https://docs.oracle.com/en/java/javase/11/troubleshoot/handle-signals-and-exceptions.html
>>> > > that USR2 is used for SUSPEND/RESUME. Is it supported by Hotspot?
>>> >
>>> > In general you have to be very careful when using signals. Yes, it can
>>> > easily break things and probably notice it quickly with debug builds as
>>> > asserts are compiled in to the builds (like the above). So I think
>>> > you've found the right page to read up on this. In this case, you can
>>> > set _JAVA_SR_SIGNUM to specify a different signal for S/R.
>>> >
>>> > -Alan