On 16/06/2022 12:09 am, Thomas Stüfe wrote:
On Wed 15. Jun 2022 at 15:06, David Holmes <david.hol...@oracle.com
<mailto:david.hol...@oracle.com>> wrote:
On 15/06/2022 8:45 pm, Thomas Stüfe wrote:
> 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.
Nor do I. SIGUSR2 is reserved for use by the VM and we document
that. If
you start throwing around random signals at processes then they can
crash - so don't do that. I'm not sure ignoring signals that shouldn't
be raised actually aids the end-user who obviously has an issue in
their
environment that needs to be addressed.
I see your point. I dislike crashes, especially ones they can be
reliably triggered from outside. You never can be sure about the
security implications either.
If you dislike ignoring the signal, and since the default action for
SIGUSR1+2 is process termination, why not exit the VM with an error
message instead? At least we don’t have what looks like a random
segfault, and we could print out the sender pid too.
We could ... but then do we do that for every other signal that can be
thrown at the JVM from externally and which will leads to crashes? Where
do you stop?
David
-----
> I mean, isn't JVM supposed to be safe? :)
Not when it comes to native code and signals - there it is more a case
of caveat emptor.
Cheers,
David
-----
> Cheers, Thomas
>
> On Wed, Jun 15, 2022 at 12:26 PM Thomas Stüfe
<thomas.stu...@gmail.com <mailto:thomas.stu...@gmail.com>
> <mailto:thomas.stu...@gmail.com
<mailto:thomas.stu...@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
> <turban...@gmail.com <mailto:turban...@gmail.com>
<mailto:turban...@gmail.com <mailto:turban...@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.bate...@oracle.com <mailto:alan.bate...@oracle.com>
<mailto:alan.bate...@oracle.com <mailto:alan.bate...@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
<https://github.com/openjdk/jdk/pull/9100#discussion_r894992558>
>
<https://github.com/openjdk/jdk/pull/9100#discussion_r894992558
<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
<https://gist.github.com/turbanoff/2099327ea13357a90df43a2d6b0e2e6a>
>
<https://gist.github.com/turbanoff/2099327ea13357a90df43a2d6b0e2e6a <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
<https://docs.oracle.com/en/java/javase/11/troubleshoot/handle-signals-and-exceptions.html>
>
<https://docs.oracle.com/en/java/javase/11/troubleshoot/handle-signals-and-exceptions.html <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
>
