RFR 8249217: Unexpected StackOverflowError in "process reaper" thread still happens

[Martin Buchholz]

+1

On Thu, Jul 23, 2020 at 3:50 PM David Holmes <david.holmes at oracle.com> wrote:
>
> Still good.
>
> Thanks,
> David
>
> On 24/07/2020 12:11 am, Roger Riggs wrote:
> > Webrev updated with Martin's suggestion:
> >
> > http://cr.openjdk.java.net/~rriggs/webrev-stackoverflow-8249217-2/
> >
> > Thanks, Roger
> >
> >
> > On 7/22/20 8:35 PM, Martin Buchholz wrote:
> >> Roger: You're absolutely right!  I should have looked.
> >>
> >> On Wed, Jul 22, 2020 at 5:25 PM Roger Riggs <Roger.Riggs at oracle.com>
> >> wrote:
> >>> Hi Martin,
> >>>
> >>> That's a good idea, but unless I miss your point, there is no Reaper
> >>> class,
> >>> only a lambda that is used as to create the Executor for the reaper
> >>> thread pool.
> >>>
> >>> Do you mean to put it before or after lines 91-93?  [1]
> >>>
> >>> Thanks, Roger
> >>>
> >>> [1]
> >>> http://cr.openjdk.java.net/~rriggs/webrev-stackoverflow-8249217/src/java.base/share/classes/java/lang/ProcessHandleImpl.java.sdiff.html
> >>>
> >>>
> >>>
> >>> On 7/22/20 8:06 PM, Martin Buchholz wrote:
> >>>> Looks good.
> >>>>
> >>>> I might move the static class preloading into a static method of the
> >>>> reaper thread class, to make it clearly the responsibility of the
> >>>> reaper thread class to enumerate and pre-load its dependencies.
> >>>>
> >>>> On Wed, Jul 22, 2020 at 7:59 AM Peter Levart
> >>>> <peter.levart at gmail.com> wrote:
> >>>>> Hi Roger,
> >>>>>
> >>>>>
> >>>>> I don't think resolving the ConcurrentHashMap.class ensures
> >>>>> initialization of the class. Just loading. But I think CHM is already
> >>>>> initialized at that time as it is used in ClassLoader to maintain
> >>>>> class
> >>>>> loading locks (per class name).
> >>>>>
> >>>>>
> >>>>> Regards, Peter
> >>>>>
> >>>>>
> >>>>> On 7/22/20 4:51 PM, Roger Riggs wrote:
> >>>>>> Please review a change to the Process reaper thread initialization to
> >>>>>> preemptively
> >>>>>> make sure classes ThreadLocalRandom and ConcurrentHashMap are
> >>>>>> initialized.
> >>>>>>   From the stack overflow failures, it seems that the classes have
> >>>>>> not
> >>>>>> been initialized
> >>>>>> before they are used during processing the termination of a process.
> >>>>>> When the initialization is performed on the smaller reaper stack, it
> >>>>>> occasionally
> >>>>>> exceeds the available stack.
> >>>>>>
> >>>>>> As an aid to diagnostics,
> >>>>>> -XX:AbortVMOnException=java.lang.StackOverflowError
> >>>>>> is added to TestHumongousNonArrayAllocation that has failed
> >>>>>> intermittently.
> >>>>>> If the problem happens again it will produce an hs_error file with
> >>>>>> useful details
> >>>>>> and will otherwise not change the test behavior.
> >>>>>>
> >>>>>> Webrev:
> >>>>>> http://cr.openjdk.java.net/~rriggs/webrev-stackoverflow-8249217/
> >>>>>>
> >>>>>> Issue:
> >>>>>>     https://bugs.openjdk.java.net/browse/JDK-8249217
> >>>>>>
> >>>>>> Thanks, Roger
> >>>>>>
> >