+1
On Thu, Jul 23, 2020 at 3:50 PM David Holmes <david.hol...@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.ri...@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.lev...@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 > >>>>>> > >