[10] RFR 8181647: jhsdb jstack could not output thread name
chihiro ito
chihiro.ito at oracle.com
Tue Jun 20 13:39:50 UTC 2017
Thank you for checking.
However, a few days ago, I have changed the output to be more similar to
jstack.
Could you possibly check for latest one?
Best regards,
Chihiro
On 2017/06/20 21:16, Jini George wrote:
> Your changes look good overall, Chihiro. A few points on the test
> case, though.
>
> * The test case checks for the existence of the compiler threads and
> the sweeper thread which would not exist when the test case is run
> with options like -Xint. You might want to remove those.
> * You might have to skip the test on OSX due to jstack issues on OSX.
> * Please correct the alignment on lines 74 and 81 for the test case.
>
> Thank you,
> Jini. (Not a (R)eviewer)
>
>
> On 6/14/2017 12:47 PM, chihiro ito wrote:
>> Hi all,
>>
>> I added a test case and modified previous patch including fix the
>> copyright year to 2017.
>> I changed to output Java thread name next the separator lines in
>> "jhsdb jstack --mixed" case as following.
>>
>> ----------------- 32117 -----------------
>> "main"
>> 0x00007f6c8feafa82 __pthread_cond_timedwait + 0x132
>>
>> Could you possibly review for this following small change? If review
>> is ok, please commit this as cito.
>>
>>
>> Source:
>> diff --git
>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
>> ---
>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
>> +++
>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All
>> rights reserved.
>> + * Copyright (c) 2003, 2017, 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
>> @@ -88,6 +88,12 @@
>> out.print("----------------- ");
>> out.print(th);
>> out.println(" -----------------");
>> + JavaThread jthread = (JavaThread) proxyToThread.get(th);
>> + if (jthread != null) {
>> + out.print("\"");
>> + out.print(jthread.getThreadName());
>> + out.println("\"");
>> + }
>> while (f != null) {
>> ClosestSymbol sym = f.closestSymbolToPC();
>> Address pc = f.pc();
>> diff --git
>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
>>
>> ---
>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
>> +++
>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2002, 2013, Oracle and/or its affiliates. All
>> rights reserved.
>> + * Copyright (c) 2002, 2017, 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
>> @@ -75,7 +75,9 @@
>> for (JavaThread cur = threads.first(); cur != null; cur
>> = cur.next(), i++) {
>> if (cur.isJavaThread()) {
>> Address sp = cur.getLastJavaSP();
>> - tty.print("Thread ");
>> + tty.print("\"");
>> + tty.print(cur.getThreadName());
>> + tty.print("\" nid=");
>> cur.printThreadIDOn(tty);
>> tty.print(": (state = " + cur.getThreadState());
>> if (verbose) {
>> diff --git a/test/serviceability/sa/JhsdbThreadNameTest.java
>> b/test/serviceability/sa/JhsdbThreadNameTest.java
>> new file mode 100644
>> --- /dev/null
>> +++ b/test/serviceability/sa/JhsdbThreadNameTest.java
>> @@ -0,0 +1,107 @@
>> +/*
>> + * Copyright (c) 2017, 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
>> + * under the terms of the GNU General Public License version 2 only, as
>> + * published by the Free Software Foundation.
>> + *
>> + * This code is distributed in the hope that it will be useful, but
>> WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of
>> MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
>> License
>> + * version 2 for more details (a copy is included in the LICENSE
>> file that
>> + * accompanied this code).
>> + *
>> + * You should have received a copy of the GNU General Public License
>> version
>> + * 2 along with this work; if not, write to the Free Software
>> Foundation,
>> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>> + *
>> + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA
>> 94065 USA
>> + * or visit www.oracle.com if you need additional information or
>> have any
>> + * questions.
>> + */
>> +
>> +import java.util.ArrayList;
>> +import java.util.Arrays;
>> +import java.util.List;
>> +import java.util.function.Consumer;
>> +
>> +import jdk.test.lib.apps.LingeredApp;
>> +import jdk.test.lib.JDKToolLauncher;
>> +import jdk.test.lib.Platform;
>> +import jdk.test.lib.process.OutputAnalyzer;
>> +import jdk.test.lib.Utils;
>> +
>> +/*
>> + * @test
>> + * @library /test/lib
>> + * @run main/othervm JhsdbThreadNameTest
>> + */
>> +public class JhsdbThreadNameTest {
>> +
>> + private static String notMixedModeThreadNames[] =
>> {"Common-Cleaner", "Signal Dispatcher", "Finalizer", "Reference
>> Handler", "main"};
>> + private static String mixedModeThreadNames[] = {"C2
>> CompilerThread0", "C1 CompilerThread1", "Sweeper thread", "Service
>> Thread"};
>> +
>> + private static void startHsdbJstack(boolean mixed) throws
>> Exception {
>> +
>> + LingeredApp app = null;
>> +
>> + try {
>> + List<String> vmArgs = new ArrayList<String>();
>> + vmArgs.add("-Xmx10m");
>> + vmArgs.addAll(Utils.getVmOptions());
>> +
>> + app = LingeredApp.startApp(vmArgs);
>> + System.out.println("Started LingeredApp with pid " +
>> app.getPid());
>> +
>> + JDKToolLauncher jhsdbLauncher =
>> JDKToolLauncher.createUsingTestJDK("jhsdb");
>> +
>> + jhsdbLauncher.addToolArg("jstack");
>> + jhsdbLauncher.addToolArg("--pid");
>> + jhsdbLauncher.addToolArg(Long.toString(app.getPid()));
>> +
>> + if (mixed) {
>> + jhsdbLauncher.addToolArg("--mixed");
>> + }
>> + ProcessBuilder pb = new ProcessBuilder();
>> + pb.command(jhsdbLauncher.getCommand());
>> + Process jhsdb = pb.start();
>> +
>> + jhsdb.waitFor();
>> +
>> + OutputAnalyzer out = new OutputAnalyzer(jhsdb);
>> +
>> +
>> Arrays.stream(notMixedModeThreadNames).map(JhsdbThreadNameTest::addQuotationMarks).forEach(out::shouldContain);
>> + Consumer<String> testMethod = null;
>> + if (mixed) {
>> + testMethod = out::shouldContain;
>> + } else {
>> + testMethod = out::shouldNotContain;
>> + }
>> +
>> Arrays.stream(mixedModeThreadNames).map(JhsdbThreadNameTest::addQuotationMarks).forEach(testMethod);
>> +
>> + } catch (InterruptedException ie) {
>> + throw new Error("Problem awaiting the child process: " +
>> ie, ie);
>> + } catch (Exception attachE) {
>> + throw new Error("Couldn't start jhsdb, attach to
>> LingeredApp or match ThreadName: " + attachE);
>> +
>> + } finally {
>> + LingeredApp.stopApp(app);
>> + }
>> + }
>> +
>> + private static String addQuotationMarks(String str) {
>> + return "\"" + str + "\"";
>> + }
>> +
>> + public static void main(String[] args) throws Exception {
>> +
>> + if (!Platform.shouldSAAttach()) {
>> + System.out.println("SA attach not expected to work -
>> test skipped.");
>> + return;
>> + }
>> +
>> + startHsdbJstack(true);
>> + startHsdbJstack(false);
>> + }
>> +}
>>
>>
>> Regards,
>> Chihiro
>>
>>
>> On 2017/06/08 18:04, chihiro ito wrote:
>>> Hi Jini,
>>>
>>> Thank you for your advices. I try to add the test case and modify
>>> the copyright year to 2017.
>>> Basically, I agree with your idea, but I think that the separator
>>> line should finally be the same as the output of the jstack command.
>>> I worry which is better way.
>>>
>>> Thanks,
>>> Chihiro
>>>
>>> On 2017/06/08 16:42, Jini George wrote:
>>>> Hi Chihiro,
>>>>
>>>> Thank you for making this useful change. Your changes look good.
>>>>
>>>> It would be great though if you could add a test case for this.
>>>> Could you also modify the copyright year to 2017 ? One additional
>>>> suggestion: The addition of the thread name makes the separator
>>>> lines unaligned in the pstack/jstack --mixed cases. Like:
>>>>
>>>> ----------------- "Service Thread" nid=16051 -----------------
>>>> and
>>>> ----------------- nid=16052 -----------------
>>>>
>>>> So I am wondering if it would make sense to have the name printed
>>>> out on a separate line to keep the separator lines aligned. But
>>>> this is a nit, and I would leave it to you to decide on this.
>>>>
>>>> Thanks,
>>>> Jini (Not a (R)eviewer)
>>>>
>>>> On 6/7/2017 3:16 PM, chihiro ito wrote:
>>>>> Hi all,
>>>>>
>>>>> I changed to output Java thread name in jhsdb jstack as following.
>>>>>
>>>>> jhsdb jstack --pid 25879
>>>>> "main" nid=25884: (state = BLOCKED)
>>>>>
>>>>> jhsdb jstack --mixed --pid 25879
>>>>> ----------------- "main" nid=25884 -----------------
>>>>>
>>>>> Could you possibly review for this following small change? If
>>>>> review is ok, please commit this as cito.
>>>>>
>>>>> Source:
>>>>> diff --git
>>>>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
>>>>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
>>>>>
>>>>> ---
>>>>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
>>>>>
>>>>> +++
>>>>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
>>>>>
>>>>> @@ -86,6 +86,13 @@
>>>>> try {
>>>>> CFrame f = cdbg.topFrameForThread(th);
>>>>> out.print("----------------- ");
>>>>> + JavaThread jthread = (JavaThread)
>>>>> proxyToThread.get(th);
>>>>> + if (jthread != null) {
>>>>> + out.print("\"");
>>>>> + out.print(jthread.getThreadName());
>>>>> + out.print("\" ");
>>>>> + }
>>>>> + out.print("nid=");
>>>>> out.print(th);
>>>>> out.println(" -----------------");
>>>>> while (f != null) {
>>>>> diff --git
>>>>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
>>>>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
>>>>>
>>>>> ---
>>>>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
>>>>> +++
>>>>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
>>>>> @@ -75,7 +75,9 @@
>>>>> for (JavaThread cur = threads.first(); cur != null;
>>>>> cur = cur.next(), i++) {
>>>>> if (cur.isJavaThread()) {
>>>>> Address sp = cur.getLastJavaSP();
>>>>> - tty.print("Thread ");
>>>>> + tty.print("\"");
>>>>> + tty.print(cur.getThreadName());
>>>>> + tty.print("\" nid=");
>>>>> cur.printThreadIDOn(tty);
>>>>> tty.print(": (state = " + cur.getThreadState());
>>>>> if (verbose) {
>>>>>
>>>>> Regards,
>>>>> Chihiro
>>>>>
>>>>
>>>
>>
>
--
Chihiro Ito | Principal Consultant | +81.90.6148.8815
Oracle <http://www.oracle.com> Consultant
ORACLE Japan | Akasaka Center Bldg. | Motoakasaka 1-3-13 | 1070051
Minato-ku, Tokyo, JAPAN
Oracle is committed to developing practices and products that help
protect the environment <http://www.oracle.com/commitment>
More information about the serviceability-dev
mailing list