Re: RFR: 8309752: com/sun/jdi/SetLocalWhileThreadInNative.java fails with virtual test thread factory due to OpaqueFrameException [v2]
> com/sun/jdi/SetLocalWhileThreadInNative.java is failing with > OpaqueFrameException when using the virtual test thread factory. The reason > is because JDI only supports calling StackFrame.setValue() on the topmost > frame of a virtual thread. The test is calling it on the > ThreadReference.frames(2), so the OpaqueFrameException is correct behavior > and the test needs to adapt. > > I could have chosen to just not have this test support running on a virtual > thread, but it appears to be the only test we have that attempts > StackFrame.setValue() on something other than the topmost frame, so it's good > to have it expect the OpaqueFrameException. > > Tested locally with and without the virtual thread wrapper. tier1 and tier5 > svc testing tbd. Chris Plummer has updated the pull request incrementally with one additional commit since the last revision: caughtOPE -> caughtOFE - Changes: - all: https://git.openjdk.org/jdk/pull/14402/files - new: https://git.openjdk.org/jdk/pull/14402/files/ee21d434..2799ebe6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14402=01 - incr: https://webrevs.openjdk.org/?repo=jdk=14402=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/14402.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14402/head:pull/14402 PR: https://git.openjdk.org/jdk/pull/14402
Re: RFR: 8309752: com/sun/jdi/SetLocalWhileThreadInNative.java fails with virtual test thread factory due to OpaqueFrameException
On Fri, 9 Jun 2023 20:47:13 GMT, Chris Plummer wrote: > com/sun/jdi/SetLocalWhileThreadInNative.java is failing with > OpaqueFrameException when using the virtual test thread factory. The reason > is because JDI only supports calling StackFrame.setValue() on the topmost > frame of a virtual thread. The test is calling it on the > ThreadReference.frames(2), so the OpaqueFrameException is correct behavior > and the test needs to adapt. > > I could have chosen to just not have this test support running on a virtual > thread, but it appears to be the only test we have that attempts > StackFrame.setValue() on something other than the topmost frame, so it's good > to have it expect the OpaqueFrameException. > > Tested locally with and without the virtual thread wrapper. tier1 and tier5 > svc testing tbd. Looks good. Thanks, Serguei test/jdk/com/sun/jdi/SetLocalWhileThreadInNative.java line 166: > 164: List localVars = frame.visibleVariables(); > 165: boolean changedLocal = false; > 166: boolean caughtOPE = false; Nit: Should it be caughtOFE instead of caughtOPE? - Marked as reviewed by sspitsyn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14402#pullrequestreview-1473268372 PR Review Comment: https://git.openjdk.org/jdk/pull/14402#discussion_r1224999636
RFR: 8309757: com/sun/jdi/ReferrersTest.java fails with virtual test thread factor
This test launches a debuggee, which creates 11 instances of its main class,
stores them in a static array of the main class, and then the debugger side
iterates over all referrers to the main class instaces. Usually this is a
pretty quick process and doesn't produce much in the way of output while
walking the reference tree of referrers. However, with virtual threads the tree
walking and output get unwieldy, and eventually it fails with:
IOException reading output of child java interpreter:Stream closed
java.lang.IllegalThreadStateException
at jdk.jdi/com.sun.tools.jdi.JDWPException.toJDIException(JDWPException.java:76)
at
jdk.jdi/com.sun.tools.jdi.ThreadReferenceImpl.name(ThreadReferenceImpl.java:197)
at
jdk.jdi/com.sun.tools.jdi.ThreadReferenceImpl.toString(ThreadReferenceImpl.java:637)
at java.base/java.lang.String.valueOf(String.java:4461)
at ReferrersTest.showReferrers(ReferrersTest.java:438)
at ReferrersTest.showReferrers(ReferrersTest.java:466)
And ReferrersTest.showReferrers() has recursed about 200 levels deep. I'm not
sure the order of these errors can be relied on. It looks like while walking
the referrers tree, the test eventually stumbled upon a thread that had exited
(but its Thread object was still alive), and this resulted in the test
aborting. If I catch these exceptions, eventually the test times out while
still working on referrers.
Judging by some of the output, it appears that introducing the TestScaffold
class as a referrer to the main debuggee class is the root cause of all these
extra referrers.
The test has a provision to cut off the recursion:
// We have to stop going up a referrer chain in some cases
Type rt = objRef.type();
if (rt instanceof ClassType) {
ClassType ct = (ClassType)rt;
String name = ct.name();
if (name.equals("sun.awt.SoftCache$ValueCell")) {
return;
}
if (name.equals("java.lang.ref.Finalizer")) {
return;
}
if (name.equals("java.lang.ref.SoftReference")) {
return;
}
// oh oh, should really check for a subclass of ClassLoader :-)
if (name.indexOf("ClassLoader") >= 0) {
return;
}
// No doubt there are other reasons to stop ...
}
Adding TestScaffold to the list makes it so the referrer tree walking output is
almost identical to what it is when not using virtual threads. Adding
java.lang.reflect.Method instead does a slightly better job.
Tested locally with and without the virtual thread wrapper. tier1 and tier5 svc
testing tbd.
-
Commit messages:
- Do some additional pruning needed when run with a virtual thread
Changes: https://git.openjdk.org/jdk/pull/14405/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=14405=00
Issue: https://bugs.openjdk.org/browse/JDK-8309757
Stats: 5 lines in 2 files changed: 3 ins; 1 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/14405.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14405/head:pull/14405
PR: https://git.openjdk.org/jdk/pull/14405
RFR: 8309752: com/sun/jdi/SetLocalWhileThreadInNative.java fails with virtual test thread factory due to OpaqueFrameException
com/sun/jdi/SetLocalWhileThreadInNative.java is failing with OpaqueFrameException when using the virtual test thread factory. The reason is because JDI only supports calling StackFrame.setValue() on the topmost frame of a virtual thread. The test is calling it on the ThreadReference.frames(2), so the OpaqueFrameException is correct behavior and the test needs to adapt. I could have chosen to just not have this test support running on a virtual thread, but it appears to be the only test we have that attempts StackFrame.setValue() on something other than the topmost frame, so it's good to have it expect the OpaqueFrameException. Tested locally with and without the virtual thread wrapper. tier1 and tier5 svc testing tbd. - Commit messages: - Fix jcheck error - Expect OpaqueFrameException when using virtual threads. Changes: https://git.openjdk.org/jdk/pull/14402/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14402=00 Issue: https://bugs.openjdk.org/browse/JDK-8309752 Stats: 11 lines in 2 files changed: 7 ins; 1 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/14402.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14402/head:pull/14402 PR: https://git.openjdk.org/jdk/pull/14402
Re: RFR: JDK-8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX
On Fri, 9 Jun 2023 13:39:26 GMT, Matthias Baesken wrote: > On AIX , new jtreg test > com/sun/tools/attach/warnings/DynamicLoadWarningTest.java always failed with > the output : > > --System.err:(294/28579)-- > STARTED DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' > SUCCESSFUL DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' > STARTED DynamicLoadWarningTest::testLoadOneJvmtiAgent '[1] > DynamicLoadWarningTest$$Lambda/0x00040020bd88@600d90bb' > org.opentest4j.AssertionFailedError: expected: <1> but was: <2> > at > org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) > at > org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) > at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197) > at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150) > at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145) > at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:528) > at > DynamicLoadWarningTest$AppRunner.stderrShouldContain(DynamicLoadWarningTest.java:298) > at > DynamicLoadWarningTest.testLoadOneJvmtiAgent(DynamicLoadWarningTest.java:125) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) > at java.base/java.lang.reflect.Method.invoke(Method.java:580) > > Reason seems to be the different behavior of dlopen on AIX compared to e.g. > Linux > This is what I find in the manpage of AIX 7.2 or 7.3 : > https://www.ibm.com/docs/en/aix/7.2?topic=d-dlopen-subroutine > https://www.ibm.com/docs/en/aix/7.3?topic=d-dlopen-subroutine > 'If the module is already loaded, it is not loaded again, but a new, unique > value will be returned by the dlopen subroutine.' > > Sounds different to what Linux documents in the manpage: > https://man7.org/linux/man-pages/man3/dlopen.3.html > 'If the same shared object is opened again with dlopen(), the same object > handle is returned.' > > We skip this special subtest on AIX . Looks good. Thanks, Serguei - Marked as reviewed by sspitsyn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14393#pullrequestreview-1472948256
Re: RFR: 8309673: Refactor ref_at methods in SA ConstantPool [v2]
On Fri, 9 Jun 2023 14:56:21 GMT, Coleen Phillimore wrote: >> Matias Saavedra Silva has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Removed unnecessary assignment > > Looks good. Thank you for the reviews @coleenp, @fparain, and @iklam ! - PR Comment: https://git.openjdk.org/jdk/pull/14385#issuecomment-1585021327
Integrated: 8309673: Refactor ref_at methods in SA ConstantPool
On Thu, 8 Jun 2023 21:42:24 GMT, Matias Saavedra Silva wrote: > The accessor methods in constantpool.cpp were previously cleaned up to allow > for different types of indices to be used, distinguishing them by the > bytecode. This patch adds the same changes to the hotspot serviceability > agent code. Verified with tier 1-5 tests. This pull request has now been integrated. Changeset: 7d6f97d0 Author:Matias Saavedra Silva URL: https://git.openjdk.org/jdk/commit/7d6f97d04d8fac44b9c71ec7e36c27ec61e82445 Stats: 95 lines in 4 files changed: 32 ins; 25 del; 38 mod 8309673: Refactor ref_at methods in SA ConstantPool Reviewed-by: coleenp, fparain, iklam - PR: https://git.openjdk.org/jdk/pull/14385
Re: RFR: 8309673: Refactor ref_at methods in SA ConstantPool [v2]
On Fri, 9 Jun 2023 18:14:18 GMT, Matias Saavedra Silva wrote: >> The accessor methods in constantpool.cpp were previously cleaned up to allow >> for different types of indices to be used, distinguishing them by the >> bytecode. This patch adds the same changes to the hotspot serviceability >> agent code. Verified with tier 1-5 tests. > > Matias Saavedra Silva has updated the pull request incrementally with one > additional commit since the last revision: > > Removed unnecessary assignment LGTM. I would suggest changing "Serviceability" in the title to "SA" - Marked as reviewed by iklam (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14385#pullrequestreview-1472921430
Re: RFR: 8309673: Refactor ref_at methods in Serviceability ConstantPool [v2]
> The accessor methods in constantpool.cpp were previously cleaned up to allow > for different types of indices to be used, distinguishing them by the > bytecode. This patch adds the same changes to the hotspot serviceability > agent code. Verified with tier 1-5 tests. Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision: Removed unnecessary assignment - Changes: - all: https://git.openjdk.org/jdk/pull/14385/files - new: https://git.openjdk.org/jdk/pull/14385/files/2ee60bd1..6630cb2c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14385=01 - incr: https://webrevs.openjdk.org/?repo=jdk=14385=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/14385.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14385/head:pull/14385 PR: https://git.openjdk.org/jdk/pull/14385
Integrated: 8232839: JDI AfterThreadDeathTest.java failed due to "FAILED: Did not get expected IllegalThreadStateException on a StepRequest.enable()"
On Wed, 7 Jun 2023 23:17:18 GMT, Chris Plummer wrote:
> The test waits for a ThreadDeathEvent for "main". Once that arrives, it then
> waits for the next ThreadStartEvent (for any thread). Once it arrives, the
> test tries to create and enable a StepRequest on the "main" thread. Since
> "main" is supposedly dead, the expectation is an IllegalThreadStateException.
> However, it turns out that sometimes the enabling can in fact succeed.
>
> Just because a ThreadDeathEvent has been received for a thread does not mean
> you can no longer do things with the thread like suspend it or enable a
> StepEvent. There is a short delay in the debug agent after sending the
> ThreadDeathEvent before it stops tracking the thread. The thread can still be
> acted upon until then. The JDWP and JDI specs seem to support doing this:
>
>> Notification of a completed thread in the target VM. The notification is
>> generated by the dying thread before it terminates. Because of this timing,
>> it is possible for {@link VirtualMachine#allThreads} to return this thread
>> after this event is received.
>>
>> Note that this event gives no information about the lifetime of the thread
>> object. It may or may not be collected soon depending on what references
>> exist in the target VM.
>
> What this means is that when the test receives some arbitrary
> ThreadStartEvent after the "main" ThreadDeathEvent has been received, the
> test may in fact still be able to enable a StepRequest on the "main" thread,
> causing the test to fail. What seems to trigger the failure is receiving an
> unexpected spurious ThreadStartEvent such as from the Common-Clean thread or
> a carrier thread, although even then it only fails some of the time. In fact
> if I modify the test to enable the StepRequest when it receives the
> ThreadDeathEvent for "main", it still almost always passes, but will fail
> more frequently than it normally does.
>
> It seems if the test always waits for the ThreadStartEvent for
> "DestroyJavaVM", then the "main" thread is truly gone by then and the test
> always passes, so this is how I've chosen to fix the issue.
>
> Tested with tier1, tier2 svc tests, and tier5 svc tests.
This pull request has now been integrated.
Changeset: 84184f94
Author:Chris Plummer
URL:
https://git.openjdk.org/jdk/commit/84184f947342fd1adbe4e3f2230ce3de4ae6007e
Stats: 24 lines in 2 files changed: 11 ins; 3 del; 10 mod
8232839: JDI AfterThreadDeathTest.java failed due to "FAILED: Did not get
expected IllegalThreadStateException on a StepRequest.enable()"
Reviewed-by: sspitsyn, kevinw
-
PR: https://git.openjdk.org/jdk/pull/14372
Re: RFR: 8232839: JDI AfterThreadDeathTest.java failed due to "FAILED: Did not get expected IllegalThreadStateException on a StepRequest.enable()"
On Wed, 7 Jun 2023 23:17:18 GMT, Chris Plummer wrote:
> The test waits for a ThreadDeathEvent for "main". Once that arrives, it then
> waits for the next ThreadStartEvent (for any thread). Once it arrives, the
> test tries to create and enable a StepRequest on the "main" thread. Since
> "main" is supposedly dead, the expectation is an IllegalThreadStateException.
> However, it turns out that sometimes the enabling can in fact succeed.
>
> Just because a ThreadDeathEvent has been received for a thread does not mean
> you can no longer do things with the thread like suspend it or enable a
> StepEvent. There is a short delay in the debug agent after sending the
> ThreadDeathEvent before it stops tracking the thread. The thread can still be
> acted upon until then. The JDWP and JDI specs seem to support doing this:
>
>> Notification of a completed thread in the target VM. The notification is
>> generated by the dying thread before it terminates. Because of this timing,
>> it is possible for {@link VirtualMachine#allThreads} to return this thread
>> after this event is received.
>>
>> Note that this event gives no information about the lifetime of the thread
>> object. It may or may not be collected soon depending on what references
>> exist in the target VM.
>
> What this means is that when the test receives some arbitrary
> ThreadStartEvent after the "main" ThreadDeathEvent has been received, the
> test may in fact still be able to enable a StepRequest on the "main" thread,
> causing the test to fail. What seems to trigger the failure is receiving an
> unexpected spurious ThreadStartEvent such as from the Common-Clean thread or
> a carrier thread, although even then it only fails some of the time. In fact
> if I modify the test to enable the StepRequest when it receives the
> ThreadDeathEvent for "main", it still almost always passes, but will fail
> more frequently than it normally does.
>
> It seems if the test always waits for the ThreadStartEvent for
> "DestroyJavaVM", then the "main" thread is truly gone by then and the test
> always passes, so this is how I've chosen to fix the issue.
>
> Tested with tier1, tier2 svc tests, and tier5 svc tests.
thanks for the reviews Kevin and Serguei!
-
PR Comment: https://git.openjdk.org/jdk/pull/14372#issuecomment-1584951868
Re: RFR: JDK-8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX
On Fri, 9 Jun 2023 13:39:26 GMT, Matthias Baesken wrote: > On AIX , new jtreg test > com/sun/tools/attach/warnings/DynamicLoadWarningTest.java always failed with > the output : > > --System.err:(294/28579)-- > STARTED DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' > SUCCESSFUL DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' > STARTED DynamicLoadWarningTest::testLoadOneJvmtiAgent '[1] > DynamicLoadWarningTest$$Lambda/0x00040020bd88@600d90bb' > org.opentest4j.AssertionFailedError: expected: <1> but was: <2> > at > org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) > at > org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) > at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197) > at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150) > at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145) > at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:528) > at > DynamicLoadWarningTest$AppRunner.stderrShouldContain(DynamicLoadWarningTest.java:298) > at > DynamicLoadWarningTest.testLoadOneJvmtiAgent(DynamicLoadWarningTest.java:125) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) > at java.base/java.lang.reflect.Method.invoke(Method.java:580) > > Reason seems to be the different behavior of dlopen on AIX compared to e.g. > Linux > This is what I find in the manpage of AIX 7.2 or 7.3 : > https://www.ibm.com/docs/en/aix/7.2?topic=d-dlopen-subroutine > https://www.ibm.com/docs/en/aix/7.3?topic=d-dlopen-subroutine > 'If the module is already loaded, it is not loaded again, but a new, unique > value will be returned by the dlopen subroutine.' > > Sounds different to what Linux documents in the manpage: > https://man7.org/linux/man-pages/man3/dlopen.3.html > 'If the same shared object is opened again with dlopen(), the same object > handle is returned.' > > We skip this special subtest on AIX . Marked as reviewed by cjplummer (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14393#pullrequestreview-1472750566
Re: RFR: 8309673: Refactor ref_at methods in Serviceability ConstantPool
On Thu, 8 Jun 2023 21:42:24 GMT, Matias Saavedra Silva wrote: > The accessor methods in constantpool.cpp were previously cleaned up to allow > for different types of indices to be used, distinguishing them by the > bytecode. This patch adds the same changes to the hotspot serviceability > code. Verified with tier 1-5 tests. Marked as reviewed by fparain (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14385#pullrequestreview-1472650636
Re: RFR: 8309673: Refactor ref_at methods in Serviceability ConstantPool
On Thu, 8 Jun 2023 21:42:24 GMT, Matias Saavedra Silva wrote: > The accessor methods in constantpool.cpp were previously cleaned up to allow > for different types of indices to be used, distinguishing them by the > bytecode. This patch adds the same changes to the hotspot serviceability > code. Verified with tier 1-5 tests. Looks good. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ConstantPool.java line 262: > 260: case Bytecodes._invokedynamic: > 261: int poolIndex = > getCache().getIndyEntryAt(index).getConstantPoolIndex(); > 262: return poolIndex = invokeDynamicNameAndTypeRefIndexAt(poolIndex); probably don't need another assignment to poolIndex in the return statement. - Marked as reviewed by coleenp (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14385#pullrequestreview-1472334909 PR Review Comment: https://git.openjdk.org/jdk/pull/14385#discussion_r1224416163
Re: RFR: JDK-8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX
On Fri, 9 Jun 2023 13:39:26 GMT, Matthias Baesken wrote: > On AIX , new jtreg test > com/sun/tools/attach/warnings/DynamicLoadWarningTest.java always failed with > the output : > > --System.err:(294/28579)-- > STARTED DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' > SUCCESSFUL DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' > STARTED DynamicLoadWarningTest::testLoadOneJvmtiAgent '[1] > DynamicLoadWarningTest$$Lambda/0x00040020bd88@600d90bb' > org.opentest4j.AssertionFailedError: expected: <1> but was: <2> > at > org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) > at > org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) > at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197) > at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150) > at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145) > at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:528) > at > DynamicLoadWarningTest$AppRunner.stderrShouldContain(DynamicLoadWarningTest.java:298) > at > DynamicLoadWarningTest.testLoadOneJvmtiAgent(DynamicLoadWarningTest.java:125) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) > at java.base/java.lang.reflect.Method.invoke(Method.java:580) > > Reason seems to be the different behavior of dlopen on AIX compared to e.g. > Linux > This is what I find in the manpage of AIX 7.2 or 7.3 : > https://www.ibm.com/docs/en/aix/7.2?topic=d-dlopen-subroutine > https://www.ibm.com/docs/en/aix/7.3?topic=d-dlopen-subroutine > 'If the module is already loaded, it is not loaded again, but a new, unique > value will be returned by the dlopen subroutine.' > > Sounds different to what Linux documents in the manpage: > https://man7.org/linux/man-pages/man3/dlopen.3.html > 'If the same shared object is opened again with dlopen(), the same object > handle is returned.' > > We skip this special subtest on AIX . Thanks for taking this one. The spec doesn't require warnings be de-duplicated when the same agent library is loaded more than once, thus skipping this test is okay where it's too complex to detect that the agent library is already loaded. - Marked as reviewed by alanb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14393#pullrequestreview-1472159054
RFR: JDK-8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX
On AIX , new jtreg test com/sun/tools/attach/warnings/DynamicLoadWarningTest.java always failed with the output : --System.err:(294/28579)-- STARTED DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' SUCCESSFUL DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' STARTED DynamicLoadWarningTest::testLoadOneJvmtiAgent '[1] DynamicLoadWarningTest$$Lambda/0x00040020bd88@600d90bb' org.opentest4j.AssertionFailedError: expected: <1> but was: <2> at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197) at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150) at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145) at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:528) at DynamicLoadWarningTest$AppRunner.stderrShouldContain(DynamicLoadWarningTest.java:298) at DynamicLoadWarningTest.testLoadOneJvmtiAgent(DynamicLoadWarningTest.java:125) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) Reason seems to be the different behavior of dlopen on AIX compared to e.g. Linux This is what I find in the manpage of AIX 7.2 or 7.3 : https://www.ibm.com/docs/en/aix/7.2?topic=d-dlopen-subroutine https://www.ibm.com/docs/en/aix/7.3?topic=d-dlopen-subroutine 'If the module is already loaded, it is not loaded again, but a new, unique value will be returned by the dlopen subroutine.' Sounds different to what Linux documents in the manpage: https://man7.org/linux/man-pages/man3/dlopen.3.html 'If the same shared object is opened again with dlopen(), the same object handle is returned.' We skip this special subtest on AIX . - Commit messages: - JDK-8309549 Changes: https://git.openjdk.org/jdk/pull/14393/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14393=00 Issue: https://bugs.openjdk.org/browse/JDK-8309549 Stats: 7 lines in 1 file changed: 3 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/14393.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14393/head:pull/14393 PR: https://git.openjdk.org/jdk/pull/14393
Re: RFR: 8309671: Avoid using jvmci.Compiler property to determine if Graal is enabled
On Fri, 9 Jun 2023 05:26:59 GMT, Serguei Spitsyn wrote: > Is JVMCI used by the Graal compiler only? So far this is true and will probably remain true for the foreseeable future. However, the Right Thing to do long term is to add a `jdk.test.whitebox.code.Compiler.uncommonTrapsHavePreciseBCIs()` method. - PR Comment: https://git.openjdk.org/jdk/pull/14381#issuecomment-1584480496
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v6]
On Thu, 8 Jun 2023 01:42:10 GMT, Serguei Spitsyn wrote: >> This is REDO the fix of >> [JDK-8307153](https://bugs.openjdk.org/browse/JDK-8307153). >> The last update of the fix in the review cycle was incorrect and incorrectly >> tested, so the issue has not been noticed. It is why the fix was backed out. >> The issue is that the SUSPEND bit was missed in the JVMTI thread state of >> platform/carrier threads carrying virtual threads >> (see`JvmtiEnvBase::get_thread_state` function). >> >> The first push/patch is the original fix of JDK-8307153. >> The fix of the SUSPEND bit issue will be in the incremental update. >> It is to simplify the review. >> >> Testing: >> - TBD: mach5 tiers 1-5 > > Serguei Spitsyn has updated the pull request incrementally with one > additional commit since the last revision: > > review: corrected the function get_thread_state for safety Chris and Alex, thank you for review! - PR Comment: https://git.openjdk.org/jdk/pull/14366#issuecomment-1584039912
Integrated: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING
On Wed, 7 Jun 2023 18:42:34 GMT, Serguei Spitsyn wrote: > This is REDO the fix of > [JDK-8307153](https://bugs.openjdk.org/browse/JDK-8307153). > The last update of the fix in the review cycle was incorrect and incorrectly > tested, so the issue has not been noticed. It is why the fix was backed out. > The issue is that the SUSPEND bit was missed in the JVMTI thread state of > platform/carrier threads carrying virtual threads > (see`JvmtiEnvBase::get_thread_state` function). > > The first push/patch is the original fix of JDK-8307153. > The fix of the SUSPEND bit issue will be in the incremental update. > It is to simplify the review. > > Testing: > - TBD: mach5 tiers 1-5 This pull request has now been integrated. Changeset: f91e9ba7 Author:Serguei Spitsyn URL: https://git.openjdk.org/jdk/commit/f91e9ba757f04983655c23542e06973805465249 Stats: 96 lines in 4 files changed: 76 ins; 0 del; 20 mod 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING Reviewed-by: cjplummer, amenkov - PR: https://git.openjdk.org/jdk/pull/14366
Re: RFR: 8309612: [REDO] JDK-8307153 JVMTI GetThreadState on carrier should return STATE_WAITING [v4]
On Thu, 8 Jun 2023 06:25:20 GMT, Alan Bateman wrote: >> It was decided with Alan that it is okay to be in a waiting state. The >> `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` state requires a monitor to be >> blocked on, so it can be confusing. Alan's comment in the original PR >> [https://github.com/openjdk/jdk/pull/14298](https://github.com/openjdk/jdk/pull/14298) >> was: >>> if the jt is carrying thread_oop and it's okay for the JVMTI state to >>> reported as WAITING when waiting for something other than Object.wait. > > The mental model is that the carrier is blocked so this is what an observer > using the APIs should see. My recollection is that JVMTI_THREAD_STATE_WAITING > was okay because there is a wriggle room in the JVM TI spec, it only uses > Object.wait as an example. There may be a few rough edges to smooth down in > this area. It's okay to take time with this PR and expand the tests to cover > more cases and get more confident that there aren't more issues. We agreed with Alex to file a test RFE to improve test coverage in this area. - PR Review Comment: https://git.openjdk.org/jdk/pull/14366#discussion_r1223883294
