On Wed, 7 Oct 2026 12:46:22 GMT, Justus Garbe <[email protected]> wrote:

>> A rare bug with the jdk has recently been filed again: 
>> https://github.com/adoptium/adoptium-support/issues/1538, which reports the 
>> already seen behavior in JDK-8352077: `unhandled instruction`. The original 
>> issue was closed with no reproduction. I have managed to write a 
>> reproduction with the new data.
>> 
>> It seems that it is caused by a missing null check in the 
>> `BreakpointInfo::set` path, which calls into:
>> `incr_number_of_breakpoints` which calls `build_method_counters`. Which has 
>> multiple ways to return `nullptr` if either the oom flag is already set or 
>> the allocation fails, which is done via allocate_no_exception since it runs 
>> on a VMThread, therefore it cant actually call a GC or raise the metadata 
>> limit. Therefore incr_number_of_breakpoints, never increases the number 
>> (since method counters is nullptr). Compilation is then paused, because the 
>> method cannot increase it's invocation counter, then after a purge caused by 
>> a gc pass, the mark is unset. After that it can correctly create it's 
>> counter, but since it never captured the breakpoints, the breakpoint is 0.
>> 
>> The method then can be picked up by a compiler thread since 
>> `number_of_breakpoints` is now 0 and a counter is real it falsely passes 
>> `Method::is_not_compilable`. Then it copies the bytecode containing the 
>> `breakpoint` instruction, and doesn't invoke the clean function since the 
>> breakpoint count reports '0'. This causes a crash in the `ciBytecodeStream` 
>> since `ciBytecodeStream::next_wide_or_table` assumes the only special 
>> bytecodes can be `wide` or a type of switch, and bytecode is a special 
>> length instruction. 
>> 
>> This also explains why this happens so rarely. For this to happen the 
>> following need to happen in succession:
>> - The debugger must try to set breakpoints
>> - The method must never have been run
>> - Metadata space needs to be at the limit and not resized when its setting 
>> the breakpoints and set the oom flag
>> - Class unloading must purge the flag afterwards
>> - The method must get compiled hot while the breakpoint is still installed 
>> 
>> 3 of the 5 conditions already happen when debugging with jdwp normally, from 
>> platforms like intellij, the others are race / resource conditions that seem 
>> happen very rarely.
>> 
>> For the fix i propose to check the method counter allocation before actually 
>> requesting a breakpoint set. Here done by checking if method counters are 
>> not null before sending the vm request. I also had to expand the user facing 
>> method to make sure the OOM error c...
>
> Justus Garbe has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add cast for result

test/hotspot/jtreg/serviceability/jvmti/SetBreakpoint/BreakpointMetaspaceOOM.java
 line 39:

> 37:  *      -XX:MaxMetaspaceSize=32m
> 38:  *      BreakpointMetaspaceOOM
> 39:  */

I have a concern about this test stability on different platforms and 
configurations. Forcing an OOME is always tricky and causing intermittent 
failures.
We may need some explicit timeout: `@run main/othervm/native/timeout=xxx`.
Also, we may need an extensive multiple (1000's) of this test runs on different 
platforms and with different VM options to prove the test is stable.

test/hotspot/jtreg/serviceability/jvmti/SetBreakpoint/libBreakpointMetaspaceOOM.cpp
 line 25:

> 23: 
> 24: #include <string.h>
> 25: #include <stdio.h>

These two includes are not needed anymore.

test/hotspot/jtreg/serviceability/jvmti/SetBreakpoint/libBreakpointMetaspaceOOM.cpp
 line 38:

> 36: static void JNICALL
> 37: Breakpoint(jvmtiEnv* env, JNIEnv* jni, jthread thread, jmethodID method, 
> jlocation location) {
> 38:   hits++;

Nit: This operation is not atomic even though the `hits` is `volatile`. I'd 
consider to use an atomic type here for safety. The probability to fail here is 
extremely low though.

test/hotspot/jtreg/serviceability/jvmti/SetBreakpoint/libBreakpointMetaspaceOOM.cpp
 line 78:

> 76: JNIEXPORT jint JNICALL
> 77: Java_BreakpointMetaspaceOOM_setBreakpoint(JNIEnv* env, jclass cls) {
> 78:   return jvmti->SetBreakpoint(target, 0);

Missed check of JVMTI error code at line 78.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/33191#discussion_r4226741088
PR Review Comment: https://git.openjdk.org/jdk/pull/33191#discussion_r4226823089
PR Review Comment: https://git.openjdk.org/jdk/pull/33191#discussion_r4226761302
PR Review Comment: https://git.openjdk.org/jdk/pull/33191#discussion_r4226692459

Reply via email to