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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a07934
fix OpaqueFrameException related issues
plummercj Jul 15, 2025
05cce17
update based on feedback
plummercj Jul 16, 2025
e637685
minor rewording
plummercj Jul 18, 2025
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
View file
Open in desktop
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
40 changes: 18 additions & 22 deletions src/jdk.jdi/share/classes/com/sun/tools/jdi/StackFrameImpl.java
View file
Open in desktop
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,25 @@ 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++) {
StackFrameImpl sf;
Copy link

Choose a reason for hiding this comment

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

There is nothing implementation-specific here.
I'd suggest to:

  • StackFrameImpl -> StackFrame;
  • MethodImpl -> Method;
  • remove validateStackFrame at line 408 ('MethodImpl.location()' calls it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting renaming the classes? This is a pretty conventional naming when you have classes implementing a spec defined in an interface class. There are a lot more than just StackFrame and Method that are doing this.

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();
}
Copy link

Choose a reason for hiding this comment

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

Are you suggesting renaming the classes?

No, I mean

Suggested change
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();
}
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();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I see now.

// 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
6 changes: 3 additions & 3 deletions src/jdk.jdi/share/classes/com/sun/tools/jdi/ThreadReferenceImpl.java
View file
Open in desktop
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