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

[Roger Riggs]

Hi,

ok.

A release note will be needed to document the new property.
Please add the label 'release-note' and a separate comment with the 
proposed release note text.

Thanks, Roger

On 2/17/2016 10:17 AM, cheleswer sahu wrote:
> 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/) 
> 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.
>>>
>