JDK-8230459: Test failed to resume JVMCI CompilerThread

[David Holmes]

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