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


Reply via email to