JDK-8230459: Test failed to resume JVMCI CompilerThread

[Vladimir Kozlov]

I resubmitted testing.

Vladimir

On 11/7/19 1:51 AM, David Holmes wrote:
> On 7/11/2019 7:08 pm, Doerr, Martin wrote:
>> Hi David,
>>
>> get_log only accesses the executing thread's own oop and the ones before it. So it's ensured by 
>> the algorithm that all accessed oops are in live handles.
> 
> Okay I see that now.
> 
> Thanks,
> David
> 
>> The problem is in can_remove when not holding the lock. For that, webrev.04 avoids accessing the 
>> oop of the last compiler thread in the case in which the lock is not held.
>>
>> Best regards,
>> Martin
>>
>>
>>> -----Original Message-----
>>> From: David Holmes <david.holmes at oracle.com>
>>> Sent: Mittwoch, 6. November 2019 11:15
>>> To: Doerr, Martin <martin.doerr at sap.com>; Kim Barrett
>>> <kim.barrett at oracle.com>
>>> Cc: dean.long at oracle.com; Vladimir Kozlov (vladimir.kozlov at oracle.com)
>>> <vladimir.kozlov at oracle.com>; hotspot-compiler-dev at openjdk.java.net
>>> Subject: Re: JDK-8230459: Test failed to resume JVMCI CompilerThread
>>>
>>> Hi Martin,
>>>
>>> On 6/11/2019 7:12 pm, Doerr, Martin wrote:
>>>> Hi Kim,
>>>>
>>>> thanks for confirming.
>>>>
>>>>
>>> http://cr.openjdk.java.net/~mdoerr/8230459_JVMCI_thread_objects/webr
>>> ev.04/
>>>> already avoids access to freed handles.
>>>
>>> Sorry I missed your earlier reference to this version.
>>>
>>> So the expectation here is that all accesses to these arrays are guarded
>>> by the CompileThread_lock, but that doesn't seem to hold for get_log ?
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>>> I don't really like the complexity of this code.
>>>> Replacing oops in handles would have been much more simple.
>>>> But I can live with either version.
>>>>
>>>> Best regards,
>>>> Martin
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Kim Barrett <kim.barrett at oracle.com>
>>>>> Sent: Mittwoch, 6. November 2019 04:09
>>>>> To: Doerr, Martin <martin.doerr at sap.com>
>>>>> Cc: David Holmes <david.holmes at oracle.com>; dean.long at oracle.com;
>>>>> Vladimir Kozlov (vladimir.kozlov at oracle.com)
>>>>> <vladimir.kozlov at oracle.com>; hotspot-compiler-dev at openjdk.java.net
>>>>> Subject: Re: JDK-8230459: Test failed to resume JVMCI CompilerThread
>>>>>
>>>>>> On Nov 5, 2019, at 3:40 AM, Doerr, Martin <martin.doerr at sap.com>
>>> wrote:
>>>>>
>>>>> Coming back in, because this seems to be going off into the weeds again.
>>>>>
>>>>>>> I don't understand what you mean. If a compiler thread holds an oop,
>>> any
>>>>>>> oop, it must hold it in a Handle to ensure it can't be gc'd.
>>>>>>
>>>>>> The problem is not related to gc.
>>>>>> My change introduces destroy_global for the handles. This means that
>>> the
>>>>> OopStorage portion which has held the oop can get freed.
>>>>>> However, other compiler threads are running concurrently. They may
>>>>> execute code which reads the oop from the handle which is freed by this
>>>>> thread.
>>>>>> Reading stale data is not a problem here, but reading freed memory may
>>>>> assert or even crash in general.
>>>>>> I can't see how OopStorage supports reading from handles which were
>>>>> freed by destroy_global.
>>>>>
>>>>> So don't do that!
>>>>>
>>>>> OopStorage isn't magic. If you are going to look at an OopStorage
>>>>> handle, you have to ensure there won't be concurrent deletion. Use
>>>>> locks or some safe memory reclamation protocol. (GlobalCounter might
>>>>> be used here, but it depends a lot on what the iterations are doing. A
>>>>> reference counting mechanism is another possibility.) This is no
>>>>> different from any other resource management.
>>>>>
>>>>>> I think it would be safe if the freeing only occurred at safepoints, but I
>>> don't
>>>>> think this is the case.
>>>>>
>>>>> Assuming the iteration didn’t happen at safepoints (which is just a way to
>>>>> make the iteration and
>>>>> deletion not concurrent).  And I agree that isn’t the case with the current
>>>>> code.
>>>>