review request for 6740526

[Daniel D. Daugherty]

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.

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