RFR[9u-dev] 8130425: libjvm crash due to stack overflow in executables with 32k tbss/tdata

cheleswer sahu cheleswer.sahu at oracle.com
Wed Feb 17 15:17:06 UTC 2016


Hi,
I have made changes in the property name 
(jdk.lang.processReaperUseDefaultStackSize) and code as suggested in the 
earlier reviews.

  --- old/src/java.base/share/classes/java/lang/ProcessHandleImpl.java 
2016-02-17 18:48:10.545639999 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java 
2016-02-17 18:48:10.081639999 +0530
@@ -81,9 +81,8 @@
                  ThreadGroup systemThreadGroup = tg;

                  ThreadFactory threadFactory = grimReaper -> {
-                    // Our thread stack requirement is quite modest.
-                    Thread t = new Thread(systemThreadGroup, grimReaper,
-                            "process reaper", 32768);
+                    long stackSize = 
Boolean.getBoolean("jdk.lang.processReaperUseDefaultStackSize") ? 0 : 32768;
+                    Thread t = new Thread(systemThreadGroup, 
grimReaper, "process reaper", stackSize);
                      t.setDaemon(true);
                      // A small attempt (probably futile) to avoid 
priority inversion
                      t.setPriority(Thread.MAX_PRIORITY);

I would really like to thanks "Martin" for the patch 
(http://cr.openjdk.java.net/~martin/webrevs/openjdk9/tls-size-guarantee/ 
<http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk9/tls-size-guarantee/>) 
which he has provided. Since we support a wider range of glibc versions 
and platform using patch will
require more testing and work. I think the use of system property for 
this issue will be safer at this point of time.

Regards,
Cheleswer


On 1/19/2016 5:40 PM, David Holmes wrote:
> On 19/01/2016 9:53 PM, Kevin Walls wrote:
>> |
>> Hi Cheleswer, I think Martin is suggesting something like:
>> |
>>
>> // Use a modest stack size, unless requested otherwise:
>> long stackSize = 
>> Boolean.getBoolean("processReaperUseDefaultStackSize") ? 0 : 32768;
>
> Someone from core-libs needs to chime on what the appropriate 
> namespace for such a property would be.
>
> David
> -----
>
>> Thread t = new Thread(systemThreadGroup, grimReaper, "process 
>> reaper", stackSize);
>>
>> |||
>> If that tests OK for you, it may be the way we can go ahead with the
>> point fix for this.
>>
>> Regards
>> Kevin
>> |
>> On 18/01/2016 16:52, Martin Buchholz wrote:
>>> Historical note - I never liked having a reaper thread for each
>>> subprocess, but no other reliable implementation is known. Given the
>>> potential for many reaper threads, I at least wanted to keep the
>>> memory waste low.
>>>
>>> On Wed, Jan 13, 2016 at 2:25 AM, cheleswer sahu
>>> <cheleswer.sahu at oracle.com> wrote:
>>>
>>>> +                   Thread t = null;
>>>> +                   if
>>>> (Boolean.getBoolean("processReaperUseDefaultStackSize")) {
>>>> +                       t = new Thread(systemThreadGroup, grimReaper,
>>>> "process reaper");
>>>> +                    } else {
>>>> +                       // Our thread stack requirement is quite 
>>>> modest.
>>>> +                       t = new Thread(systemThreadGroup, grimReaper,
>>>> "process reaper", 32768);
>>>> +                    }
>>> If we do end up using this strategy, cleaner would be to use
>>> https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#Thread-java.lang.ThreadGroup-java.lang.Runnable-java.lang.String-long- 
>>>
>>> stackSize - the desired stack size for the new thread, or zero to
>>> indicate that this parameter is to be ignored.
>>

-------------- next part --------------
--- old/src/java.base/share/classes/java/lang/ProcessHandleImpl.java	2016-02-17 18:48:10.545639999 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java	2016-02-17 18:48:10.081639999 +0530
@@ -81,9 +81,8 @@
                 ThreadGroup systemThreadGroup = tg;
 
                 ThreadFactory threadFactory = grimReaper -> {
-                    // Our thread stack requirement is quite modest.
-                    Thread t = new Thread(systemThreadGroup, grimReaper,
-                            "process reaper", 32768);
+                    long stackSize = Boolean.getBoolean("jdk.lang.processReaperUseDefaultStackSize") ? 0 : 32768;
+                    Thread t = new Thread(systemThreadGroup, grimReaper, "process reaper", stackSize); 
                     t.setDaemon(true);
                     // A small attempt (probably futile) to avoid priority inversion
                     t.setPriority(Thread.MAX_PRIORITY);


More information about the hotspot-runtime-dev mailing list