On Thu, 30 Apr 2026 02:35:35 GMT, David Holmes <[email protected]> wrote:
>> Please review this change to fix a problem that can rise if JVM TI >> suspension is allowed when a thread is executing in a JNI "critical" region. >> The gory details are in the first comment so that the PR emails are shorter >> >> A new test is introduced to check that we cannot suspend in a critical region >> >> Other testing: >> - Tiers 1-5 on all platforms >> >> The key insights into this solution are attributed to @pchilano. Everything >> simpler I tried was buggy and led me back to Patricio's suggested changes to >> the operation filtering. The actual details of this and any remaining bugs >> in it are all my own. >> >> Thanks. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > David Holmes has updated the pull request incrementally with one additional > commit since the last revision: > > Fix typo src/hotspot/share/runtime/javaThread.hpp line 980: > 978: void exit_jni_deferred_suspension() { > 979: assert(Thread::current() == this, "this must be current thread"); > 980: _jni_deferred_suspension_count--; Why `AtomicAccess::inc(&_jni_deferred_suspension_count);` is not used here? (Same for dec()) I am not sure I see how this access is protected. src/hotspot/share/runtime/javaThread.inline.hpp line 195: > 193: } > 194: > 195: void JavaThread::enter_jni_deferred_suspension() { Why `enter_jni_deferred_suspension()` is in this file and `jni_deferred_suspension()` with `enter_jni_deferred_suspension()` are in other file? Also, why enter_jni_deferred_suspension has assertion and this method precond? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30936#discussion_r3171854093 PR Review Comment: https://git.openjdk.org/jdk/pull/30936#discussion_r3171921033
