Skip to content

8309400: JDI spec needs to clarify when OpaqueFrameException and NativeMethodException are thrown #26335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/jdk.jdi/share/classes/com/sun/jdi/ThreadReference.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -402,8 +402,8 @@ List<MonitorInfo> ownedMonitorsAndFrames()
* @throws java.lang.IllegalArgumentException if <CODE>frame</CODE>
* is not on this thread's call stack.
*
* @throws OpaqueFrameException if this thread is a suspended virtual thread and the
* target VM was unable to pop the frames.
* @throws OpaqueFrameException if target VM is unable to pop this frame
* (e.g. a virtual thread is not suspended at an event).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'm not sure the comment at line 406 is fully correct. The IncompatibleThreadStateException will be thrown if a virtual thread is not suspended. So, the OpaqueFrameException will be thrown only when a virtual thread is suspended but not at an event. The same concern should apply for the line 488.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are talking about the difference between "not suspended" vs "not suspended at an event". The former results in IncompatibleThreadStateException. For the latter I felt the wording implied it was suspended, but not at an event. Maybe it should just say "suspended, but not at an event"?

The JVMTI spec does not have this issue because it doesn't mention virtual threads as part of the OPAQUE_ERROR description. It instead mentions the method being native as an example of what can cause OPAQUE_ERROR. We can't do that here because a native method results in NativeMethodException, not OpaqueFrameException.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it should just say "suspended, but not at an event"?

Yes, this would make it clear.
And yes, the JVMTI vs JDI spec difference is reasonable.

*
* @throws NativeMethodException if one of the frames that would be
* popped is that of a native method or if the frame previous to
Expand Down Expand Up @@ -484,8 +484,8 @@ List<MonitorInfo> ownedMonitorsAndFrames()
* @throws IncompatibleThreadStateException if this
* thread is not suspended.
*
* @throws OpaqueFrameException if this thread is a suspended virtual thread and the
* target VM is unable to force the method to return.
* @throws OpaqueFrameException if target VM is unable to pop this frame
* (e.g. a virtual thread is not suspended at an event).
*
* @throws NativeMethodException if the frame to be returned from
* is that of a native method.
Expand Down
39 changes: 17 additions & 22 deletions src/jdk.jdi/share/classes/com/sun/tools/jdi/StackFrameImpl.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -395,29 +395,24 @@ public PacketStream send() {
} catch (JDWPException exc) {
switch (exc.errorCode()) {
case JDWP.Error.OPAQUE_FRAME:
if (thread.isVirtual()) {
// We first need to find out if the current frame is native, or if the
// previous frame is native, in which case we throw NativeMethodException
for (int i = 0; i < 2; i++) {
StackFrameImpl sf;
try {
sf = (StackFrameImpl)thread.frame(i);
} catch (IndexOutOfBoundsException e) {
// This should never happen, but we need to check for it.
break;
}
sf.validateStackFrame();
MethodImpl meth = (MethodImpl)sf.location().method();
if (meth.isNative()) {
throw new NativeMethodException();
}
// We first need to find out if the current frame is native, or if the
// previous frame is native, in which case we throw NativeMethodException
for (int i = 0; i < 2; i++) {
StackFrame sf;
try {
sf = thread.frame(i);
} catch (IndexOutOfBoundsException e) {
// This should never happen, but we need to check for it.
break;
}
Method meth = sf.location().method();
if (meth.isNative()) {
throw new NativeMethodException();
}
// No native frames involved. Must have been due to thread
// not being mounted.
throw new OpaqueFrameException();
} else {
throw new NativeMethodException();
}
// No native frames involved. Must have been due to virtual thread
// not being mounted or some other reason such as failure to deopt.
throw new OpaqueFrameException();
case JDWP.Error.THREAD_NOT_SUSPENDED:
throw new IncompatibleThreadStateException(
"Thread not current or suspended");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,10 +597,10 @@ public void forceEarlyReturn(Value returnValue) throws InvalidTypeException,
} catch (JDWPException exc) {
switch (exc.errorCode()) {
case JDWP.Error.OPAQUE_FRAME:
if (isVirtual() && !meth.isNative()) {
throw new OpaqueFrameException();
} else {
if (meth.isNative()) {
throw new NativeMethodException();
} else {
throw new OpaqueFrameException();
}
case JDWP.Error.THREAD_NOT_SUSPENDED:
throw new IncompatibleThreadStateException(
Expand Down