RFR (jdk10): JDK-8180175: ObjectSynchronizer only needs to iterate in-use monitors

[Robbin Ehn]

Hi,

On 05/18/2017 10:10 AM, David Holmes wrote:
> On 18/05/2017 6:08 PM, Roman Kennke wrote:
>> Am 18.05.2017 um 10:03 schrieb David Holmes:
>>> On 18/05/2017 5:54 PM, Roman Kennke wrote:
>>>> Am 18.05.2017 um 04:07 schrieb David Holmes:
>>>>> Roman,
>>>>>
>>>>> I just pushed this but now I think there is a problem with this
>>>>> optimization.
>>>>>
>>>>> When a thread terminates, omFlush is used to transfer the thread's
>>>>> inUseList to the global inUseList. This happens after the thread has
>>>>> been removed from the _threads_list and can run concurrently with a
>>>>> safepoint. This means that there is a window of time where the
>>>>> monitors in the thread's inUseList will not be seen by the safepoint
>>>>> oops_do processing:
>>>>> - the thread oops_do won't be called because the thread is not in the
>>>>> thread's list
>>>>> - the monitors are not yet in the global in-use-list
>>>>>
>>>>> Did I overlook something?

Good catch!

>>>>
>>>> Ugh. How ugly :-)
>>>>
>>>> Is there a reason why omFlush cannot be called from the same block where
>>>> the thread is removed from _thread_list, under the Threads_lock ?

Couldn't omFlush be done before Threads_lock in Thread::remove, while the thread is still on threads list?
I do not see the reason why you want to hold the Threads_lock? To me it looks like gListLock is enough?

/Robbin

>>>
>>> I think the explicit intent is to let it run independent of safepoints
>>> (which holding the Threads_lock would affect).
>>
>> But Thread::remove() *is* going to block on Threads_lock. If I read the
>> code correctly, omFlush() is called right after we give up Threads_lock
> 
> Right - in the spirit of "critical sections should be as short as possible" the omFlush is done outside of the holding of the threads_lock.
> 
>> (thread_main_inner()). Stuff under gListLock in omFlush() is only doing
>> some simple re-linking of the linked lists. I'll give that a try with
>> some very careful checking ...
> 
> It's the other uses of the gListlock that need to be examined.
> 
> David
> 
>> Roman
>>