RFR(s): 8209139: globalCounter bootstrap issue

[David Holmes]

Hi Robbin,

On 7/11/2018 8:01 PM, Robbin Ehn wrote:
> Hi David,
> 
> On 2018-11-07 10:40, David Holmes wrote:
>> Hi Robbin,
>>
>> Can't you just use an _initialized flag ?? 
> 
> I can, but I need to changes many places since the ThreadsList is not 
> expected
> to be null. And I do not see how checking that flag instead checking if 
> the list
> is the bootstrap would be better.

 From the bug there seemed to be only once place that needed this check. 
Where are all these other places?

> Or statically initialize
>> _java_thread_list to a zero-length list? 
> 
> It's statically initialized to _bootstrap_list. Since I'm going to need 
> to check
> for a bootstrap list in a non-SMR-thread I can't follow the pointer 
> because we
> can crash. So I need to know which address to check for, this is the
> _bootstrap_list. And since it's static I can't free it.

Not following. Can't you initialize _java_thread_list directly to a 
zero-length list and that will "just work" because you'll iterate zero 
items? There will only ever be one zero-length threads-list right?

Thanks,
David
-----

>> This seems overly elaborate.
> 
> If we get a SMR bug doing it different could give us wired crashes 
> instead of
> clean assert failures. That is why you think this is 'overly elaborate' 
> I guess.
> 
>>
>> I'm not even concerned about the existing "hack" - checking for 
>> "Threads::number_of_threads() == 0" has always been a simple way to 
>> see if you are very early in the init sequence.
> 
> Well it only solves the problem for the GlobalCounter.
> And in theory we could have added non-JavaThreads to the RCU thread-set 
> they
> should be checked.
> 
> Thanks, Robbin
> 
>>
>> Thanks,
>> David
>>
>> On 7/11/2018 6:56 PM, Robbin Ehn wrote:
>>> Hi all, please review.
>>>
>>> When changing the hashtable for SymbolTable we did a hack to handle 
>>> the case
>>> when symbols gets inserted before the VM is proper initialized, by 
>>> doing nothing
>>> in the GlobalCounter if there is no threads. The problem is that the 
>>> thread
>>> creating the VM is not yet added to the VM (non-SMR-thread) and thus
>>> ThreadsListHandle do not yet work for that thread. Trying to move the 
>>> addition
>>> of the creating thread earlier in the bootstrap is troublesome.
>>>
>>> This removes that hack and do a more generic solution for ThreadsList 
>>> instead.
>>> To make it safe to verify that we are talking about the bootstrap 
>>> ThreadsList from a non-SMR-thread we cannot follow that pointer. 
>>> Using null was intrusive to the code I therefore choose to do a 
>>> static bootstrap ThreadsList.
>>>
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8209139
>>> Webrev: http://cr.openjdk.java.net/~rehn/8209139/webrev/
>>>
>>> Tested with use of ThreadsListHandle and GlobalCounter in the 
>>> problematic init method, passed t1-2 with use of ThreadsListHandle in 
>>> that init method.
>>>
>>> Thanks, Robbin