JDK-8230459: Test failed to resume JVMCI CompilerThread

[David Holmes]

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.
>>>