[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: code review request for deadlock detection fix (8007476)
From:       "Daniel D. Daugherty" <daniel.daugherty () oracle ! com>
Date:       2013-02-28 3:09:47
Message-ID: 512ECA7B.8060901 () oracle ! com
[Download RAW message or body]

David,

Thanks for the review. I suspect that I'll have to resubmit the
JPRT job due to a conflicting job in another queue and I'll update
the reviewer list if I do so.

Dan

On 2/27/13 6:40 PM, David Holmes wrote:
> Hi Dan,
>
> This looks fine to me. I'm a little nervous the removal of the asserts 
> as we should detect when an owning thread disappears as it is a major 
> problem. But if those asserts are only in the diagnostic paths then it 
> is better to let the diagnostics complete as you have done.
>
> On 28/02/2013 12:52 AM, Karen Kinnear wrote:
>> Code looks good. I did not study the details (again) for the monitor 
>> owner, but
>> my memory is that there are other cases in which the monitor owner 
>> can be
>> temporarily null, so this is will help there also.
>
> Karen, it isn't that the monitor owner is temporarily null, but that 
> the thread associated with an owned monitor can not be found. That 
> should never happen. Whenever a monitor has an owner the "owner" 
> information should lead to a live thread.
>
> David
> -----
>
>>
>> thanks,
>> Karen
>>
>> On Feb 26, 2013, at 6:54 PM, Daniel D. Daugherty wrote:
>>
>>> Greetings,
>>>
>>> I have a fix to make deadlock detection a little more robust in the 
>>> case
>>> of being unable to find the JavaThread associated with an object lock:
>>>
>>>     8007476 assert(the_owner != NULL) failed: Did not find owning Java
>>>             thread for lock word address
>>>     http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8007476
>>>     https://jbs.oracle.com/bugs/browse/JDK-8007476
>>>
>>> Yes, I know that's not supposed to happen, but if and when it does 
>>> happen,
>>> it would be good to be able to diagnose the VM to try and figure out 
>>> what
>>> the heck happened.
>>>
>>> Here is the webrev URL:
>>>
>>>     http://cr.openjdk.java.net/~dcubed/8007476-webrev/0-hsx25/
>>>
>>> There is some sample output in the bug report that shows both regular
>>> deadlock detection output and the proposed modified deadlock detection
>>> output.
>>>
>>> Thanks, in advance, for any comments, suggestions or questions.
>>>
>>> Dan
>>>
>>> P.S.
>>> Yes, I've included a bit of tweaking to JVM/TI code to deal with
>>> invalid assertion failures that are discussed in the following bug:
>>>
>>>     6786879 Race in jvmti GetObjectMonitorUsage could lead to crashes
>>>     http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6786879
>>>     https://jbs.oracle.com/bugs/browse/JDK-6786879
>>>
>>> However, I haven't tried to address all the races that are discussed
>>> in that bug including the race in the call to
>>> JvmtiEnv::is_thread_fully_suspended() where the underlying call to
>>> JavaThread::wait_for_ext_suspend_completion() can crash on lock exit.
>>> Yikes, that one is nasty.
>>

[prev in list] [next in list] [prev in thread] [next in thread]