[10] RFR 8181647: jhsdb jstack could not output thread name
Jini George
jini.george at oracle.com
Tue Jun 20 12:16:45 UTC 2017
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
>>>>
>>>
>>
>
More information about the serviceability-dev
mailing list