[11u] RFR: 8257828 SafeFetch may crash if invoked in non-JavaThreads

[Thomas Stüfe]

Thank you Martin!

On Tue, May 11, 2021 at 1:02 PM Doerr, Martin <martin.doerr at sap.com> wrote:

> Hi Thomas,
>
>
>
> looks good to me. Thanks for fixing it in 11u.
>
>
>
> Best regards,
>
> Martin
>
>
>
>
>
> *Von: *hotspot-runtime-dev <hotspot-runtime-dev-retn at openjdk.java.net> im
> Auftrag von Thomas Stüfe <thomas.stuefe at gmail.com>
> *Datum: *Dienstag, 11. Mai 2021 um 08:08
> *An: *jdk-updates-dev <jdk-updates-dev at openjdk.java.net>
> *Cc: *Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>
> *Betreff: *[11u] RFR: 8257828 SafeFetch may crash if invoked in
> non-JavaThreads
>
> Hi,
>
> I'd like reviews for this downport, please.
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8257828
> Original patch: https://git.openjdk.java.net/jdk/commit/3ab1dfeb
> Original Review: https://git.openjdk.java.net/jdk/pull/1695
>
> This patch does two things:
>
> 1) On Posix platforms it fixes SafeFetch to work in all threads, not only
> in JavaThreads. In non-JavaThreads before this patch it would crash.
>
> The patch is totally trivial. In the signal handler, when handling
> SafeFetch, there was a condition that Thread != NULL and be a JavaThread.
> This condition was unnecessary. All SafeFetch handling needs is a valid
> context. The Patch moves SafeFetch handling out of the is-JavaThread
> condition.
>
> 2) On Windows we did not have the problem; however there is the
> `Handle_Exception` function handling SafeFetch among other things, which
> may be called with a non-java thread, but then forcibly casts the Thread*
> to JavaThread.
>
> 11u patch:
>
> http://cr.openjdk.java.net/~stuefe/webrevs/backports/8257828-safefetch-non-java-threads
>
> The original patch does not apply, at all. The reason for this is that we
> consolidated common functionality of the many cpu+arch signal handlers to
> one function, in os_posix.cpp. This happened in JDK16 and I do not want to
> backport this cleanup change to 11. For this patch, it means that the
> posix-related fixes practically had to be rewritten for each and every
> signal handler. Each fix is primitive however, since it just moves
> SafeFetch handling around, but every signal handler is a tiny bit
> different.
>
> I tested this patch for several weeks in our systems on Linux
> ppc/ppcle/x64/s390/aarch64, AIX, Solaris and Mac, no problems surfaced.
>
> I did not test on 32bit arm.
>
> Thanks, Thomas
>