RFR: JDK-8210252: com/sun/jdi/DebuggerThreadTest.java fails with NPE

Gary Adams gary.adams at oracle.com
Thu Sep 13 11:57:24 UTC 2018


Patch attached.

On 9/12/18, 3:42 PM, Chris Plummer wrote:
> Looks good.
>
> Chris
>
> On 9/12/18 8:20 AM, Gary Adams wrote:
>> Here's the updated webrev to avoid the NPE, but still
>> fail the test with more information about the premature
>> completed threads.
>>
>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8210252
>>   Webrev: http://cr.openjdk.java.net/~gadams/8210252/webrev.00/
>>
>> On 9/5/18, 3:16 PM, Gary Adams wrote:
>>> After a quick local check of finishedThreads =1,
>>> I moved the check til after the full list of threads is checked,
>>> rather than repeating the stderr failure after each entry was
>>> processed.
>>>
>>> I also implemented a resumeVM() in TestScaffold and called it before
>>> dumpThreads(). That did not fail locally, so I submitted a mach5 job 
>>> so I can
>>> run more tests in batches.
>>>
>>> ...
>>>
>>> On 9/5/18, 2:56 PM, Chris Plummer wrote:
>>>> I was thinking something like the resume() call. At the very least 
>>>> just try setting finishedThreads = 1 to exercise the error handling.
>>>>
>>>> Chris
>>>>
>>>>  On 9/5/18 11:49 AM, Gary Adams wrote:
>>>>> I had tried sleeping after each entry in dumpThreads was checked, 
>>>>> but the
>>>>> failure never occurred.
>>>>>
>>>>> I had tried using mach5 --jvm-args to set UseZGC, but that never 
>>>>> failed.
>>>>>
>>>>> Could attempt a resume() before calling dumpThreads() - haven't 
>>>>> tried that , yet.
>>>>>
>>>>> On 9/5/18, 2:31 PM, Chris Plummer wrote:
>>>>>> Ok. Have you found a way to test your changes? Maybe just force 
>>>>>> or simulate a failure somehow.
>>>>>>
>>>>>> Chris
>>>>>>
>>>>>> On 9/5/18 11:26 AM, Gary Adams wrote:
>>>>>>> If we go ahead with this proposed fix, the next time it fails
>>>>>>> we will have more information about the other threads
>>>>>>> captured in the log.
>>>>>>>
>>>>>>> One last observation, the test that failed was running with UseZGC.
>>>>>>> Not sure how that might impact this particular test.
>>>>>>>
>>>>>>> On 9/5/18, 2:07 PM, Chris Plummer wrote:
>>>>>>>>
>>>>>>>> Hi Gary,
>>>>>>>>
>>>>>>>>> If the DebugThreadTarg is resumed prematurely, 
>>>>>>>>
>>>>>>>> The failure explanation seems to hinge on this, but I don't see 
>>>>>>>> how this can happen. What is resuming DebugThreadTarg? Only the 
>>>>>>>> call to listenUntilVMDisconnect() should be doing this, and we 
>>>>>>>> aren't even getting that far. The only thing I can think of is 
>>>>>>>> due to some other issue (maybe host out of memory so process 
>>>>>>>> was killed off), the DebugThreadTarg process has exited 
>>>>>>>> unexpectedly. In this case I guess the fix you have is 
>>>>>>>> appropriate defensive programming, causing the test to fail 
>>>>>>>> when this happens, but still won't explain why it happens and 
>>>>>>>> also won't prevent future failures of this test. Also, this 
>>>>>>>> type of Debuggee early exit problem could happen with other 
>>>>>>>> tests too. I'm leaning towards just closing this bug as CNR. I 
>>>>>>>> don't think we want to be writing our tests to defend against 
>>>>>>>> random system related failures, especially when the end result 
>>>>>>>> is still going to be a test failure that we don't fully 
>>>>>>>> understand.
>>>>>>>>
>>>>>>>> thanks,
>>>>>>>>
>>>>>>>> Chris
>>>>>>>>
>>>>>>>> On 9/5/18 7:33 AM, Gary Adams wrote:
>>>>>>>>> The DebuggerThreadTest ensures it is in sync
>>>>>>>>> at the beginning of RunTests with a breakpoint event
>>>>>>>>> in DebuggerThreadTarg ready() method.
>>>>>>>>>
>>>>>>>>> The DebuggerThreadTest then continues with
>>>>>>>>> dumpThreads() and listenUntilVMDisconnect()
>>>>>>>>> completes.
>>>>>>>>>
>>>>>>>>> If the DebugThreadTarg is resumed prematurely,
>>>>>>>>> the main thread in the debuggee could complete
>>>>>>>>> before the dumpThreads enumeration is complete.
>>>>>>>>>
>>>>>>>>> DebuggerThreadTest
>>>>>>>>>   main()
>>>>>>>>>     startTests()
>>>>>>>>>        runTests()
>>>>>>>>>           startTo( "DebuggerThreadTarg.ready()")
>>>>>>>>>           dumpThreads()
>>>>>>>>>           listenUntilVMDisconnect()
>>>>>>>>>
>>>>>>>>> DebuggerThreadTarg
>>>>>>>>>   main()
>>>>>>>>>      ready()
>>>>>>>>>
>>>>>>>>> Revised fix:
>>>>>>>>>   - Prevents the NPE from a finished thread group
>>>>>>>>>   - Fails the test with a message indicating
>>>>>>>>>      number of premature completed threads.
>>>>>>>>>
>>>>>>>>> diff --git a/test/jdk/com/sun/jdi/DebuggerThreadTest.java 
>>>>>>>>> b/test/jdk/com/sun/jdi/DebuggerThreadTest.java
>>>>>>>>> --- a/test/jdk/com/sun/jdi/DebuggerThreadTest.java
>>>>>>>>> +++ b/test/jdk/com/sun/jdi/DebuggerThreadTest.java
>>>>>>>>> @@ -1,5 +1,5 @@
>>>>>>>>>  /*
>>>>>>>>> - * Copyright (c) 2001, 2015, Oracle and/or its affiliates. 
>>>>>>>>> All rights reserved.
>>>>>>>>> + * Copyright (c) 2001, 2018, 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
>>>>>>>>> @@ -76,8 +76,16 @@
>>>>>>>>>          Thread list[] = new Thread[listThreads * 2];
>>>>>>>>>          int gotThreads = tg.enumerate(list, true);
>>>>>>>>>          for (int i = 0; i < Math.min(gotThreads, 
>>>>>>>>> list.length); i++){
>>>>>>>>> +            int finishedThreads = 0;
>>>>>>>>>              Thread t = list[i];
>>>>>>>>> -            String groupName = t.getThreadGroup().getName();
>>>>>>>>> +            ThreadGroup tga = t.getThreadGroup();
>>>>>>>>> +            String groupName;
>>>>>>>>> +            if (tga == null) {
>>>>>>>>> +                groupName = "<completed>";
>>>>>>>>> +                finishedThreads++ ;
>>>>>>>>> +            } else {
>>>>>>>>> +                groupName =tga.getName();
>>>>>>>>> +            }
>>>>>>>>>
>>>>>>>>>              System.out.println("Thread [" + i + "] group = '" +
>>>>>>>>>                                 groupName +
>>>>>>>>> @@ -89,7 +97,10 @@
>>>>>>>>>                  failure("FAIL: non-daemon thread '" + 
>>>>>>>>> t.getName() +
>>>>>>>>>                          "' found in ThreadGroup '" + 
>>>>>>>>> groupName + "'");
>>>>>>>>>              }
>>>>>>>>> -
>>>>>>>>> +            if (finishedThreads > 0 ) {
>>>>>>>>> +                failure("FAIL: " + finishedThreads +
>>>>>>>>> +                        " threads completed while VM 
>>>>>>>>> suspended.");
>>>>>>>>> +            }
>>>>>>>>>          }
>>>>>>>>>      }
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 9/4/18, 3:15 PM, Chris Plummer wrote:
>>>>>>>>>> Hi Gary,
>>>>>>>>>>
>>>>>>>>>> The failed case only had:
>>>>>>>>>>
>>>>>>>>>> Thread [0] group = 'system' name = 'Reference Handler' daemon 
>>>>>>>>>> = true
>>>>>>>>>> Thread [1] group = 'system' name = 'Finalizer' daemon = true
>>>>>>>>>> Thread [2] group = 'system' name = 'Signal Dispatcher' daemon 
>>>>>>>>>> = true
>>>>>>>>>> Thread [3] group = 'system' name = 'process reaper' daemon = 
>>>>>>>>>> true
>>>>>>>>>>
>>>>>>>>>> That would indicate that 'main' is likely the thread that 
>>>>>>>>>> exited. Seems odd. Isn't that the thread that the test is 
>>>>>>>>>> executing in?
>>>>>>>>>>
>>>>>>>>>> If you can't reproduce it, maybe it would be better to commit 
>>>>>>>>>> a diagnostic fix like the one I suggested and keep an eye on 
>>>>>>>>>> it. However, it only seems to have failed once due to this 
>>>>>>>>>> reason, so unless it is a new problem we may never see it again.
>>>>>>>>>>
>>>>>>>>>> Chris
>>>>>>>>>>
>>>>>>>>>> On 9/4/18 11:28 AM, Gary Adams wrote:
>>>>>>>>>>> I haven't been able to reproduce the problem locally.
>>>>>>>>>>> Trying larger test runs on mach5 now.
>>>>>>>>>>>
>>>>>>>>>>> Here's the output from a successful test run.
>>>>>>>>>>> If any of the threads exited, they would have a null group 
>>>>>>>>>>> name.
>>>>>>>>>>>
>>>>>>>>>>> Howdy!
>>>>>>>>>>> Thread [0] group = 'system' name = 'Reference Handler' 
>>>>>>>>>>> daemon = true
>>>>>>>>>>> Thread [1] group = 'system' name = 'Finalizer' daemon = true
>>>>>>>>>>> Thread [2] group = 'system' name = 'Signal Dispatcher' 
>>>>>>>>>>> daemon = true
>>>>>>>>>>> Thread [3] group = 'system' name = 'process reaper' daemon = 
>>>>>>>>>>> true
>>>>>>>>>>> Thread [4] group = 'main' name = 'main' daemon = false
>>>>>>>>>>> Thread [5] group = 'main' name = 'pool-1-thread-1' daemon = 
>>>>>>>>>>> true
>>>>>>>>>>> Thread [6] group = 'AgentVMThreadGroup' name = 
>>>>>>>>>>> 'AgentVMThread' daemon = false
>>>>>>>>>>> Thread [7] group = 'AgentVMThreadGroup' name = 'output 
>>>>>>>>>>> reader' daemon = false
>>>>>>>>>>> Thread [8] group = 'AgentVMThreadGroup' name = 'output 
>>>>>>>>>>> reader' daemon = false
>>>>>>>>>>> Thread [9] group = 'AgentVMThreadGroup' name = 'Thread-5' 
>>>>>>>>>>> daemon = true
>>>>>>>>>>> Thread [10] group = 'InnocuousThreadGroup' name = 
>>>>>>>>>>> 'Common-Cleaner' daemon = true
>>>>>>>>>>> Thread [11] group = 'JDI [1485331767]' name = 'JDI Internal 
>>>>>>>>>>> Event Handler' daemon = true
>>>>>>>>>>> Thread [12] group = 'JDI [1485331767]' name = 'JDI Target VM 
>>>>>>>>>>> Interface' daemon = true
>>>>>>>>>>> Goodbye from DebuggerThreadTarg!
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 9/4/18, 2:16 PM, Chris Plummer wrote:
>>>>>>>>>>>> Can you reproduce the problem? If so, maybe to find out 
>>>>>>>>>>>> which thread is a problem you could check for null, print 
>>>>>>>>>>>> the thread info, and then fail the test.
>>>>>>>>>>>>
>>>>>>>>>>>> Chris
>>>>>>>>>>>>
>>>>>>>>>>>> On 9/4/18 11:14 AM, Gary Adams wrote:
>>>>>>>>>>>>> I'm not sure which thread exited causes the NPE.
>>>>>>>>>>>>> This patch will let  the test continue and at least
>>>>>>>>>>>>> let the list of threads be processed.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The test walks up the parents to the initial thread
>>>>>>>>>>>>> and then "enumerates()" the set of the threads to check.
>>>>>>>>>>>>> There is an inherent race condition in enumerate()
>>>>>>>>>>>>> that recognizes it is a snapshot of a moving target.
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 9/4/18, 1:51 PM, Chris Plummer wrote:
>>>>>>>>>>>>>> Hi Gary,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Why has the thread exited if the debuggee is still running?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Chris
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 9/4/18 5:22 AM, Gary Adams wrote:
>>>>>>>>>>>>>>> Here's a quick fix to avoid the NPE using a 
>>>>>>>>>>>>>>> getThreadGroup() which could be null
>>>>>>>>>>>>>>> if the thread has terminated.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8210252
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> diff --git 
>>>>>>>>>>>>>>> a/test/jdk/com/sun/jdi/DebuggerThreadTest.java 
>>>>>>>>>>>>>>> b/test/jdk/com/sun/jdi/DebuggerThreadTest.java
>>>>>>>>>>>>>>> --- a/test/jdk/com/sun/jdi/DebuggerThreadTest.java
>>>>>>>>>>>>>>> +++ b/test/jdk/com/sun/jdi/DebuggerThreadTest.java
>>>>>>>>>>>>>>> @@ -77,7 +77,8 @@
>>>>>>>>>>>>>>>          int gotThreads = tg.enumerate(list, true);
>>>>>>>>>>>>>>>          for (int i = 0; i < Math.min(gotThreads, 
>>>>>>>>>>>>>>> list.length); i++){
>>>>>>>>>>>>>>>              Thread t = list[i];
>>>>>>>>>>>>>>> - String groupName = t.getThreadGroup().getName();
>>>>>>>>>>>>>>> + ThreadGroup tga = t.getThreadGroup();
>>>>>>>>>>>>>>> + String groupName = (tga == null ? "<completed>": 
>>>>>>>>>>>>>>> tga.getName());
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>              System.out.println("Thread [" + i + "] 
>>>>>>>>>>>>>>> group = '" +
>>>>>>>>>>>>>>> groupName + 
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>
>

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 8210252.patch
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180913/ee0a8ac8/8210252-0001.patch>


More information about the serviceability-dev mailing list