JDK-8230459: Test failed to resume JVMCI CompilerThread

[Vladimir Kozlov]

On 11/12/19 7:40 AM, Doerr, Martin wrote:
> Hi Vladimir et al.,
> 
> thanks a lot for running these tests. I've seen that they have passed.
> I haven't got any more comments, so should I push the latest version?
> Are you ok with it?

Yes, please push it.

Vladimir

> 
> Best regards,
> Martin
> 
> 
>> -----Original Message-----
>> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
>> Sent: Donnerstag, 7. November 2019 23:15
>> To: David Holmes <david.holmes at oracle.com>; Doerr, Martin
>> <martin.doerr at sap.com>; Kim Barrett <kim.barrett at oracle.com>
>> Cc: dean.long at oracle.com; hotspot-compiler-dev at openjdk.java.net
>> Subject: Re: JDK-8230459: Test failed to resume JVMCI CompilerThread
>>
>> 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.
>>>>>>