review request for 6740526

[Xiaobin Lu]

Daniel D. Daugherty wrote:
> There is already some special casing for the Terminator_lock
> relative to the Safepoint_lock. Perhaps there is need for a
> more complete special case for the Terminator_lock. Or perhaps
> we need to have the ability to make orthogonal locks and then
> morph all the existing special case locks into orthogonal locks.
>
> I guess what I'm saying is that we need to revisit use of the
> Terminator_lock and its interaction with other locks.
Hi Dan,

I think it might be better to log a RFE for this feature and address 
this particular crash first.

-Xiaobin
>
> Dan
>
>
> David Holmes - Sun Microsystems wrote:
>> Hi Xiaobin,
>>
>> I'd add to the comment:
>>
>> "The right way to prevent termination would be to acquire the 
>> Terminator_lock, but we can't do that without violating the lock rank 
>> checking in some cases."
>>
>> Cheers,
>> David
>>
>> Xiaobin Lu said the following on 08/23/08 11:51:
>>> Webrev: http://javaweb.sfbay/~xl116366/webrev/6740526/
>>>
>>> 6740526:  sun/management/HotspotThreadMBean/GetInternalThreads.java 
>>> test failed
>>>
>>> Details:
>>>
>>> The above test failure is caused by a recent fix for 6608862. The 
>>> problem there was that when some JVMTI operation was made to examine 
>>> all the threads' state and while in the meantime, the WatcherThread 
>>> is actually exiting. This makes the ThreadClosure->do_thread call 
>>> fail since we are now referencing a NULL pointer. To preserve 
>>> WatcherThread and its data inside memory, we grab the 
>>> Terminator_lock before making a call on it. So the thread calling 
>>> WatcherThread::stop will be blocked and therefore can't set the 
>>> reference to NULL and destroy the structure. I assumed that these 
>>> operations (basically Threads::threads_do) are always called at 
>>> safepoint, therefore, the rank checking when acquiring the 
>>> Terminator_lock will be bypassed. The assumption is wrong since 
>>> obviously some calls originating from management.cpp for example, 
>>> aren't issued at safepoint, thus making the rank checking fail.
>>>
>>> The fix is that we remove the lock acquisition. As documented in the 
>>> comment, strictly speaking, this isn't sufficient to make sure the 
>>> data for WatcherThread is still there when the call is actually 
>>> being made, but the chance of that is extremely small.
>>>
>>> Reviewed by:
>>>
>>> Verified by:
>>> All tests under sun/management
>>> Test reported in 6608862
>>> PRT
>>>
>>> Thanks in advance!
>>>
>>> -Xiaobin