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

Chris Plummer chris.plummer at oracle.com
Wed Sep 12 19:42:17 UTC 2018 

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 + 
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>

More information about the serviceability-dev mailing list