On Wed, 29 Oct 2025 17:58:40 GMT, Jorn Vernee <[email protected]> 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:
>
> Remove requires vm.debug
Functional changes seem fine to me - this seems a reasonable approach to take.
Tests seem okay (one nit) but I'm not that familiar with testing in this area.
Thanks.
PS. I will be likely be on vacation before this finalizes.
test/jdk/java/foreign/sharedclosejvmti/TestSharedCloseJvmti.java line 65:
> 63: Process process = ProcessTools.startProcess("fork", pb, null,
> null, 1L, TimeUnit.MINUTES);
> 64: OutputAnalyzer output = new OutputAnalyzer(process);
> 65: assertEquals(0, output.getExitValue());
Suggestion:
output.shouldHaveExitValue(0);
That way we get the diagnostic summary printed on failure.
-------------
Marked as reviewed by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/27919#pullrequestreview-3397321596
PR Review Comment: https://git.openjdk.org/jdk/pull/27919#discussion_r2476317588
