JDK-8230459: Test failed to resume JVMCI CompilerThread

[Doerr, Martin]

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?

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