JDK-8230459: Test failed to resume JVMCI CompilerThread

[Doerr, Martin]

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