<div><br></div><div dir="auto">Okay, sure. I thought you referred to the way the VM relies in undefined signal delivery specifics for signals sent to the process.</div><div dir="auto"><br><div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">On Wed 15. Jun 2022 at 13:17, Andrey Turbanov <<a href="mailto:turbanoff@gmail.com">turbanoff@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1ex;border-left-color:rgb(204,204,204)">>not C++ undefined behavior, nothing to do with C++<br>
<br>
>From 'signals_posix.cpp':<br>
<br>
Thread* thread = Thread::current_or_null_safe();<br>
assert(thread != NULL, "Missing current thread in SR_handler");<br>
<br>
if (thread->has_terminated()) {<br>
  return;<br>
}<br>
<br>
In release build there will be no assert and 'thread' will be 'NULL'.<br>
This is C++ code and in C++ dereferencing of NULL pointers is<br>
undefined behaviour.<br>
Do I miss something?<br>
<br>
<br>
<br>
<br>
Andrey Turbanov<br>
<br>
ср, 15 июн. 2022 г. в 13:45, Thomas Stüfe <<a href="mailto:thomas.stuefe@gmail.com" target="_blank">thomas.stuefe@gmail.com</a>>:<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> Cheers, Thomas<br>
><br>
> On Wed, Jun 15, 2022 at 12:26 PM Thomas Stüfe <<a href="mailto:thomas.stuefe@gmail.com" target="_blank">thomas.stuefe@gmail.com</a>> wrote:<br>
>><br>
>> 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.<br>
>><br>
>> 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).<br>
>><br>
>> 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.<br>
>><br>
>> If nobody objects, I can file an issue and prepare the patch.<br>
>><br>
>> Cheers, Thomas<br>
>><br>
>> (P.s. not C++ undefined behavior, nothing to do with C++ :-)<br>
>><br>
>> On Wed, Jun 15, 2022 at 12:11 PM Andrey Turbanov <<a href="mailto:turbanoff@gmail.com" target="_blank">turbanoff@gmail.com</a>> wrote:<br>
>>><br>
>>> I mean, isn't JVM supposed to be safe? :)<br>
>>> Receiving this signal _could_ happen in a real deployment. And now, as<br>
>>> I can see, we have C++ undefined behaviour in release builds in this<br>
>>> case. Can we consider this as a bug?<br>
>>><br>
>>> Andrey Turbanov<br>
>>><br>
>>> вт, 14 июн. 2022 г. в 14:46, Alan Bateman <<a href="mailto:Alan.Bateman@oracle.com" target="_blank">Alan.Bateman@oracle.com</a>>:<br>
>>> ><br>
>>> > On 14/06/2022 10:44, Andrey Turbanov wrote:<br>
>>> > > Hello.<br>
>>> > > During investigation of signal handling in JVM (for<br>
>>> > > <a href="https://github.com/openjdk/jdk/pull/9100#discussion_r894992558" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/pull/9100#discussion_r894992558</a> )<br>
>>> > > I found out that sending USR2 crashes my JDK. (Linux fastdebug x64)<br>
>>> > ><br>
>>> > > kill -USR2 1346792<br>
>>> > ><br>
>>> > > # assert(thread != __null) failed: Missing current thread in SR_handler<br>
>>> > > # Internal Error<br>
>>> > > (/home/turbanoff/Projects/official_jdk/src/hotspot/os/posix/signals_posix.cpp:1600),<br>
>>> > > pid=1346792, tid=1346792<br>
>>> > ><br>
>>> > > Full hs_err_pid1346792.log:<br>
>>> > > <a href="https://gist.github.com/turbanoff/2099327ea13357a90df43a2d6b0e2e6a" rel="noreferrer" target="_blank">https://gist.github.com/turbanoff/2099327ea13357a90df43a2d6b0e2e6a</a><br>
>>> > ><br>
>>> > ><br>
>>> > > Is it known/expected behaviour?<br>
>>> > > I found some description there<br>
>>> > > <a href="https://docs.oracle.com/en/java/javase/11/troubleshoot/handle-signals-and-exceptions.html" rel="noreferrer" target="_blank">https://docs.oracle.com/en/java/javase/11/troubleshoot/handle-signals-and-exceptions.html</a><br>
>>> > > that USR2 is used for SUSPEND/RESUME. Is it supported by Hotspot?<br>
>>> ><br>
>>> > In general you have to be very careful when using signals. Yes, it can<br>
>>> > easily break things and probably notice it quickly with debug builds as<br>
>>> > asserts are compiled in to the builds (like the above). So I think<br>
>>> > you've found the right page to read up on this. In this case, you can<br>
>>> > set _JAVA_SR_SIGNUM to specify a different signal for S/R.<br>
>>> ><br>
>>> > -Alan<br>
</blockquote></div></div>