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

[Kevin Walls]

Thanks Martin - I think we're going to proceed with the arguably 
somewhat ugly change as it does solve a real problem, although somewhat 
unusual to have such large thread local sizes for it to matter.  And we 
may have to deal with a thread library that takes away stack space for 
thread local data for a while.


Thanks
Kevin


On 25/02/2016 16:30, Martin Buchholz wrote:

My hope is that we eventually eliminate StackOverflowError by
implementing dynamically growable thread stacks.

Until then, my hope is that hotspot give the thread the memory it asks
for, for actually storing stack frames, and thread locals should not
steal space from that.

The system property introduced in this change is an ugly workaround for that.

On Thu, Feb 18, 2016 at 2:24 AM, Kevin Walls  wrote:

Hi Cheleswer,

Looks good to me.

Thanks
Kevin
(Also, as one of the comments was that there may be no real cost to using
default stack sizes here (on most systems...?), having
jdk.lang.processReaperUseDefaultStackSize gives us a way to test that
widely, and one day the 32k may be able to disappear.)





On 17/02/2016 15:17, 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.54563 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
2016-02-17 18:48:10.08163 +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
  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.

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

[Andrew Haley]

On 02/25/2016 04:30 PM, Martin Buchholz wrote:
> My hope is that we eventually eliminate StackOverflowError by
> implementing dynamically growable thread stacks.

Well, we've got this anyway, at least in Linux.  Isn't this something
that naturally should be pushed down to the thread library?

Andrew.

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

[Martin Buchholz]

My hope is that we eventually eliminate StackOverflowError by
implementing dynamically growable thread stacks.

Until then, my hope is that hotspot give the thread the memory it asks
for, for actually storing stack frames, and thread locals should not
steal space from that.

The system property introduced in this change is an ugly workaround for that.

On Thu, Feb 18, 2016 at 2:24 AM, Kevin Walls  wrote:
> Hi Cheleswer,
>
> Looks good to me.
>
> Thanks
> Kevin
> (Also, as one of the comments was that there may be no real cost to using
> default stack sizes here (on most systems...?), having
> jdk.lang.processReaperUseDefaultStackSize gives us a way to test that
> widely, and one day the 32k may be able to disappear.)
>
>
>
>
>
> On 17/02/2016 15:17, 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.54563 +0530
> +++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
> 2016-02-17 18:48:10.08163 +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
>   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.
>
>
>
>

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

[David Holmes]

For the record this looks fine to me too.

Thanks,
David
-

On 18/02/2016 1: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.54563 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
2016-02-17 18:48:10.08163 +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
 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.

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

[Kevin Walls]

Hi Cheleswer,

Looks good to me.

Thanks
Kevin
(Also, as one of the comments was that there may be no real cost to 
using default stack sizes here (on most systems...?), having 
jdk.lang.processReaperUseDefaultStackSize gives us a way to test that 
widely, and one day the 32k may be able to disappear.)





On 17/02/2016 15:17, 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.54563 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java 
2016-02-17 18:48:10.08163 +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
 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.

Re: 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.54563 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java 
2016-02-17 18:48:10.08163 +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
 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.

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

[cheleswer sahu]

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.54563 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java 
2016-02-17 18:48:10.08163 +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
 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.




--- old/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
2016-02-17 18:48:10.54563 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
2016-02-17 18:48:10.08163 +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);

Re: 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
  wrote:


+   Thread t = null;
+   if
(Boolean.getBoolean("processReaperUseDefaultStackSize")) {
+   t = new Thread(systemThreadGroup,
grimReaper,
"process reaper");
+} else {
+   // Our thread stack requirement is quite

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

[David Holmes]

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


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
  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.

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

[Andrew Haley]

On 01/20/2016 03:19 PM, Peter Levart wrote:
> 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. 

I'm not sure how many thread-local variables are possible in theory,
but I suspect it's unlimited.  In machines with growable memory
regions for stacks allocating the default stack has very little
overhead, so it doesn't matter.  Systems which pre-allocate their
thread stacks have their own problems.  These tend to be special kinds
of cloud containers and some turnkey systems which cannot tolerate an
out-of-memory error.

Andrew.

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

[Peter Levart]

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.


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
  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.

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

[Roger Riggs]

Hi Peter,

David, on serviceability-dev [1], was of the opinion that a simple 
increase would be too temporary.


Roger

[1]


On 1/20/2016 10: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.


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
  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.

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

[Roger Riggs]

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

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
  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.

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

[Kevin Walls]

|
Hi Cheleswer, I think Martin is suggesting something like:
|

// Use a modest stack size, unless requested otherwise:
long stackSize = Boolean.getBoolean("processReaperUseDefaultStackSize") ? 0 : 
32768;
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
 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.

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

[David Holmes]

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
  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.

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

[David Holmes]

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 ...


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
  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.

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

[David Holmes]

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
  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.

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

[Roger Riggs]

Hi,

How about "jdk.process.reaperStackSize".

It will need a CCC .

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
  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.

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

[Martin Buchholz]

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
 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.

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

[David Holmes]

On 15/01/2016 4:59 AM, Martin Buchholz wrote:

The process grim reaper ends up being the first point of failure since
it tries not to waste the user's memory and it's in a core library,
but in principle it's not special.  I think a more general workaround
would be to add a hotspot flag that would add a memory safety zone to
all threads.  If it's known that TLS is stealing 32k from every
thread's stack, then the flag should ensure that every thread stack is
32k larger.


I think we need a point fix for the process reaper case in the immediate 
term. We can then consider how to better address the general case in the 
medium to long term.



More generally, I was hoping that hotspot would ensure that the -Xss
size was available for actual java stack frames and OS overhead was
added on automatically, but that is hard to implement, so the best
alternative workaround is for users to be able to specify that
additional stack size padding.  Maybe -XX:StackSizeOverhead=32768 ?


Even this is still a band-aid - the user has to experience the problem, 
recognize it, and then figure out the right adjustment to add. Plus if 
any third-party library uses native TLS the requirement could change 
dynamically.


So I'd prefer a new RFE to look at this general issue.

Thanks,
David

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

[Martin Buchholz]

The process grim reaper ends up being the first point of failure since
it tries not to waste the user's memory and it's in a core library,
but in principle it's not special.  I think a more general workaround
would be to add a hotspot flag that would add a memory safety zone to
all threads.  If it's known that TLS is stealing 32k from every
thread's stack, then the flag should ensure that every thread stack is
32k larger.

More generally, I was hoping that hotspot would ensure that the -Xss
size was available for actual java stack frames and OS overhead was
added on automatically, but that is hard to implement, so the best
alternative workaround is for users to be able to specify that
additional stack size padding.  Maybe -XX:StackSizeOverhead=32768 ?

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

[cheleswer sahu]

Adding core-libs-dev and hotspot-runtime-dev team .

On 1/14/2016 12:24 AM, Martin Buchholz wrote:

You should include core-libs-dev (and perhaps hotspot-runtime-dev) in
this thread.  You're changing core library code.

On Wed, Jan 13, 2016 at 2:25 AM, cheleswer sahu
 wrote:

Hi,

Please review the code changes for
"https://bugs.openjdk.java.net/browse/JDK-8130425;.

Problem summary: When a large TLS (Thread Local Storage) size is set for
threads,  JVM is throwing StackOverflow exception.

Problem Identified:
As per investigation and a discussion we came to the conclusion that issue
is not with the JVM but it lies in the way glibc has been implemented. When
a TLS is declared , it steals the space from threads stack size.
So if a thread is created with small stack size, and TLS is set to a large
value, then it will result in StackOverflow. This is the exact case in this
bug where reaper thread is allocated a very low stack size 32768.

Discussion thread:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037558.html

Solution proposed:
Its expected to get fix in glibc sometime , but for now I propose a
workaround, a boolean system property "processReaperUseDefaultStackSize"
using which we can set the stack size for reaper thread to default
instead of fix 32768. This property can be set by the user using "-D" or
"System.setProperty()". I have tested this fix, it works well with TLS size
between 32k to 128k.

Fix:
diff -r 5c4530bb9ae6
src/java.base/share/classes/java/lang/ProcessHandleImpl.jav
--- a/src/java.base/share/classes/java/lang/ProcessHandleImpl.java Fri Jan
08 13:06:29 2016 +0800
+++ b/src/java.base/share/classes/java/lang/ProcessHandleImpl.java Tue Jan
12 15:55:50 2016 +0530
@@ -83,9 +83,13 @@
  ThreadGroup systemThreadGroup = tg;

  ThreadFactory threadFactory = grimReaper -> {
-// Our thread stack requirement is quite modest.
-Thread t = new Thread(systemThreadGroup, grimReaper,
-"process reaper", 32768);
+   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);
+}
  t.setDaemon(true);
  // A small attempt (probably futile) to avoid priority
inversion
  t.setPriority(Thread.MAX_PRIORITY);


Regards,
Cheleswer