On Thu, 23 Jan 2025 09:07:23 GMT, Matthias Baesken <mbaes...@openjdk.org> wrote:
>> We were running into this error :
>> sun/tools/jhsdb/HeapDumpTestWithActiveProcess.java on Linux aarch64, jdk25
>>
>> stderr: [java.lang.RuntimeException: Unable to deduce type of thread from
>> address 0x0000ffff34144e30 (expected type JavaThread, CompilerThread,
>> MonitorDeflationThread, AttachListenerThread, StringDedupThread,
>> NotificationThread, ServiceThread or JvmtiAgentThread)
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.runtime.Threads.createJavaThreadWrapper(Threads.java:196)
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.runtime.Threads.getJavaThreadAt(Threads.java:178)
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.utilities.HeapHprofBinWriter.dumpStackTraces(HeapHprofBinWriter.java:828)
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.utilities.HeapHprofBinWriter.write(HeapHprofBinWriter.java:460)
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.tools.JMap.writeHeapHprofBin(JMap.java:216)
>> at jdk.hotspot.agent/sun.jvm.hotspot.tools.JMap.run(JMap.java:103)
>> at jdk.hotspot.agent/sun.jvm.hotspot.tools.Tool.startInternal(Tool.java:278)
>> at jdk.hotspot.agent/sun.jvm.hotspot.tools.Tool.start(Tool.java:241)
>> at jdk.hotspot.agent/sun.jvm.hotspot.tools.Tool.execute(Tool.java:134)
>> at jdk.hotspot.agent/sun.jvm.hotspot.tools.JMap.main(JMap.java:202)
>> at jdk.hotspot.agent/sun.jvm.hotspot.SALauncher.runJMAP(SALauncher.java:344)
>> at jdk.hotspot.agent/sun.jvm.hotspot.SALauncher.main(SALauncher.java:507)
>> Caused by: sun.jvm.hotspot.types.WrongTypeException: No suitable match for
>> type of address 0x0000ffff34144e30
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.runtime.InstanceConstructor.newWrongTypeException(InstanceConstructor.java:62)
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.runtime.VirtualConstructor.instantiateWrapperFor(VirtualConstructor.java:80)
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.runtime.Threads.createJavaThreadWrapper(Threads.java:192)
>> ... 11 more
>>
>> In the discussion in the JBS bug it was found that a retry option in` launch
>> `should be added.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> check doSleep in launch
It's functionally correct but could use some comments.
test/jdk/sun/tools/jhsdb/JShellHeapDumpTest.java line 56:
> 54: static boolean doSleep = true; // By default do a short sleep when
> app starts up
> 55:
> 56: public static boolean launch(String expectedMessage, List<String>
> toolArgs, boolean allowRetry)
You should add a comment explaining the return value like we have for
printStackTraces().
test/jdk/sun/tools/jhsdb/JShellHeapDumpTest.java line 105:
> 103:
> 104: boolean res = launch(expectedMessage, Arrays.asList(toolArgs),
> true);
> 105: if (!res && !doSleep) {
You should explain why we allow a retry here, and only for !doSleep, like we do
in printStackTraces(). Note the doSleep reason is a bit different here. It is
because the sleep allows the debuggee to stabilize, making it very unlikely
that jmap will fail.
-------------
Changes requested by cjplummer (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/23213#pullrequestreview-2579815041
PR Review Comment: https://git.openjdk.org/jdk/pull/23213#discussion_r1933131474
PR Review Comment: https://git.openjdk.org/jdk/pull/23213#discussion_r1933133351
