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

[Peter Levart]

On 01/21/2016 09:57 AM, David Holmes wrote:
> The cc's are going dropped unfortunately - adding by serviceability.
>
> On 21/01/2016 1:19 AM, Peter Levart wrote:
>> Hi,
>>
>> On 01/20/2016 03:09 PM, Roger Riggs wrote:
>>> Hi David,
>>>
>>> I read an old description;  I was expecting the value of the property
>>> to be the stack size for the reaper.
>>> That would allow more control and less waste when it needed to be
>>> overridden.
>>> Set it to zero if the default size is desired.
>>>
>>> Roger
>>
>> Wouldn't it be more appropriate that for modest use there would be no
>> need to specify a system property. By modest use, I mean starting just a
>> few sub-processes. So if we add 32k (or whatever the max. possible
>> overhead of TLS is) to the existing 32k, that should be the default for
>> reaper thread. If one needs to make it even smaller to accommodate for
>> exceptional use, he can then use this new system property.
>
> There is no reasonable "max possible" - the 32K is from the current 
> problem that needs to be solved. But simply adding 32K today doesn't 
> help if the third-part-library increases it's TLS use to 33K next 
> week. We need a means for the person running the JVM to expand the 
> stack size of the reaper thread if they hit this TLS-based problem. A 
> flag that says "use the Java Thread default" suffices because the Java 
> stack size can be further adjusted if even its default is not enough.
>
> Or we could just say forget about trying to manage this one thread's 
> stack and let it behave like a normal Java thread. Might be the 
> simplest solution by far. As Martin said he was just trying to be a 
> good citizen with limiting the potential memory use.
>
> Cheers,
> David

Ok, I see.

But what about a more "complicated" ;-) solution, like:


public class ProbeMinThreadStackSize {

     static class ProbeThread extends Thread {
         ProbeThread(Runnable target, long stackSize) {
             super(Thread.currentThread().getThreadGroup(),
                   target,
                   "ProbeThread-stackSize-" + stackSize,
                   stackSize);
         }

         boolean failed;

         @Override
         public void run() {
             try {
                 super.run();
             } catch (Throwable e) {
                 failed = true;
             }
         }
     }

     static long probeMinStackSize(long min, long step, long max, 
Runnable probeCode) {
         for (long ss = min; ss <= max; ss += step) {
             try {
                 ProbeThread pt = new ProbeThread(probeCode, ss);
                 pt.start();
                 pt.join();
                 if (!pt.failed) {
                     // got it!
                     return ss;
                 }
             } catch (Throwable t) {
                 // continue with next
             }
         }
         // return 0 indicating usage of default Thread's stack size
         return 0L;
     }

     static void recurse(int n, long p1, long p2, long p3, long p4) {
         if (n > 0) recurse(n-1, p2, p3, p4, p1);
     }

     public static void main(String[] args) {
         long minStackSize = probeMinStackSize(32768L, 8192L, 1024L*1024L,
                                               () -> recurse(100, 1L, 
2L, 3L, 4L));
         System.out.println(minStackSize);
     }
}



Regards, Peter

>
>> Regards, Peter
>>
>>>
>>> On 1/20/2016 12:05 AM, David Holmes wrote:
>>>> On 20/01/2016 8:39 AM, David Holmes wrote:
>>>>> On 20/01/2016 12:50 AM, Roger Riggs wrote:
>>>>>> Hi,
>>>>>>
>>>>>> How about "jdk.process.reaperStackSize".
>>>>>
>>>>> Based on some internal discussion that seems appropriate to me ...
>>>>
>>>> Except of course that the property is just a boolean not a size. :)
>>>> So jdk.process.reaperUsesdefaultStackSize seems appropriate.
>>>>
>>>> David
>>>>
>>>>>> It will need a CCC .
>>>>>
>>>>> ... and if not then the CCC will show the way. :)
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Roger
>>>>>>
>>>>>>
>>>>>> On 1/19/16 7:10 AM, 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.
>>>>>>>>
>>>>>>
>>>
>>