RFR: 8370344: Arbitrary Java frames on stack during scoped access [v5]

[Patricio Chilano Mateo]

On Thu, 30 Oct 2025 22:47:41 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> See the JBS issue for a problem description.
>> 
>> This patch changes the shared scope closure handshake to be able to handle 'arbitrary' Java frames on the stack during a scoped memory access.
>> 
>> For the purposes of this change, we assume that 'arbitrary' is limited to the following: 
>> 1. Frames added by calling the constructor of `InternalError` as a result of a faulting access to a truncated memory-mapped file (see `HandshakeState::handle_unsafe_access_error`). This is the only handshake operation (i.e. may be triggered during a scoped access) that calls back into Java.
>> 2. Frames added by a JVMTI agent that calls back into Java code while handling a JVMTI event that happens during a scoped access.
>> 3. Any other Java code that runs as part of the linking process.
>> 
>> For (1), we set a flag while we are create the `InternalError`. If a thread has that flag set, we know it is in the process of crashing already, so we don't have to inspect the stack at all. For (2), all bets are off, so we have to walk the entire stack. For (3), this patch switches the hard limit of 10 frames for the stack walk to instead bail out at the first frame outside of the `java.base` module. In most cases this speeds up the stack walk as well, if threads are running other code.
>> 
>> The test `TestSharedCloseJFR` is added for scenario (1), and the test `TestSharedCloseJvmti` is added for scenario (2). Existing tests already cover scenario (3).
>> 
>> Testing: tier 1-4
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update bug number in tests
>   
>   Co-authored-by: Chen Liang <liach at openjdk.org>

Fix looks good to me. Only have some comments about the JVMTI agent case, thanks.

src/hotspot/share/prims/scopedMemoryAccess.cpp line 53:

> 51: #endif
> 52: 
> 53:   bool agents_loaded = JvmtiAgentList::has_agents();

I see that for dynamically loaded agents we add to the list after loading the agent. Maybe we should check `JvmtiEnvBase::environments_might_exist()`?

src/hotspot/share/prims/scopedMemoryAccess.cpp line 94:

> 92: 
> 93: static bool is_accessing_session(JavaThread* jt, oop session, bool& in_scoped) {
> 94:   if (jt->is_throwing_unsafe_access_error()) {

If we assume arbitrary Java code in JVMTI callbacks this might return true but the target could be in a different nested scoped access. I think we should check we are in the no agent case before bailing out.

src/hotspot/share/runtime/javaThread.hpp line 1364:

> 1362:   JavaThread* _thread;
> 1363: public:
> 1364:   ThrowingUnsafeAccessError(JavaThread* thread) : _thread(thread) {

If we assume arbitrary Java code in JVMTI callbacks this could be executed recursively and `_throwing_unsafe_access_error` be set to false while we are within the outer caller. Although it’s fine since we will do a full stack walk in `is_accessing_session`, we should add a comment why this recursive case is okay (or save the old value as with `UnlockFlagSaver`).

-------------

PR Review: https://git.openjdk.org/jdk/pull/27919#pullrequestreview-3411931601
PR Review Comment: https://git.openjdk.org/jdk/pull/27919#discussion_r2487147887
PR Review Comment: https://git.openjdk.org/jdk/pull/27919#discussion_r2487150214
PR Review Comment: https://git.openjdk.org/jdk/pull/27919#discussion_r2487152847