On 15/04/2019 5:34 pm, Patrick Zhang OS wrote:
Removed it. http://cr.openjdk.java.net/~qpzhang/8222334/webrev.03/jdk.changeset
By the way, could you please sponsor to push it once approved? thanks in
advance.
Sure - if the core-libs person who also reviews doesn't volunteer (hint
hint ;-) )
Cheers,
David
Regards
Patrick
-----Original Message-----
From: David Holmes <david.hol...@oracle.com>
Sent: Monday, April 15, 2019 2:33 PM
To: Patrick Zhang OS <patr...@os.amperecomputing.com>; core-libs-dev
<core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8222334: java -Xss0 triggers StackOverflowError
Hi Patrick,
On 15/04/2019 3:42 pm, Patrick Zhang OS wrote:
Hi David,
Many thanks, I integrated your updates into the new patch.
Thanks. My only further comment is to not have:
947 * See JDK-8222334 for details
Cross references from code to bug reports should be very rare and I don't think
this one warrants it. No need to see updated webrev in that case.
Cheers,
David
I think STACK_SIZE_MINIMUM is an empirical value, NOT suitable for 'all' platforms, at least not
that safe, for example, a tricky experiment is: create the initial thread with 320K, and have later
VM inner threads created with 448K, on my aarch64 system, StackOverflowError would be thrown.
Fortunately this probably would not occur in real cases, as -Xss60k, -Xss320k, etc. can be stopped
by these if-clauses. I changed "all platforms" to "most platforms".
"only used for windows" might ambiguously mean "GetDefaultJavaVMInitArgs returns 0 for windows
only" or "only windows supports 0". I updated it as "for example, Windows"
http://cr.openjdk.java.net/~qpzhang/8222334/webrev.02/
Regards
Patrick
-----Original Message-----
From: David Holmes <david.hol...@oracle.com>
Sent: Monday, April 15, 2019 6:55 AM
To: Patrick Zhang OS <patr...@os.amperecomputing.com>; core-libs-dev
<core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8222334: java -Xss0 triggers StackOverflowError
Hi Patrick,
Please see:
http://cr.openjdk.java.net/~dholmes/8222334/webrev/
for my suggested updates to the commentary. Note that GetDefaultJavaVMInitArgs returns
the build-time default stack sizes and so will only return 0 (for "use the system
default") on Windows. It is not affected by -XX:ThreadStackSize=n as that only gets
processed when the JVM is actually loaded.
Thanks,
David
On 12/04/2019 6:11 pm, David Holmes wrote:
Hi Patrick,
First apologies that it took me so long to get my head around this.
:) Let me summarise the problem as I see it.
The launcher specifies no particular semantics for -Xss0, to it 0 is
just a very small size. However the VM maps -Xss to
-XX:ThreadStackSize and for it 0 means "use the platform default stack size".
The launcher examines -Xss because it needs to use it to define the
stacksize for the initial thread created to launch the VM.
The VM examines -Xss to see what stacksize to use for subsequently
created threads and it treats 0 as 'use the platform default' and it
otherwise checks the value against some hardcoded minimums and
reports an error if it is too small.
The initial thread that loads the VM needs sufficient stack to be
able to process things to the point where it can determine that the
requested stacksize is too small and report the error. The value of
the minimum stack is hardcoded into the launcher, as
STACK_SIZE_MINIMUM (64KB). If the -Xss value is less than that then it gets set
to that.
If no -Xss is specified then the launcher asks the VM for a
reasonable value to use for the stacksize of the initial thread (typically 1MB).
The problem arises with -Xss0 because this causes the launcher to set
an initial thread stacksize of STACK_SIZE_MINIMUM, but the VM sees
this as "use the default" and so does not reject it and tries to
continue with VM initialization. That can't succeed as we only have a
tiny STACK_SIZE_MINIMUM stack and so we get StackOverflowError (or
fail an assert in debug builds).
So the solution, as Patrick proposes, is to treat -Xss0 in the
launcher as-if -Xss has not been set and so use the VM suggested
default for the initial thread's stacksize.
So I agree with the functional change here, but have some alternate
suggestions for additional commentary. Unfortunately I have to step
away at the moment (its Friday night) so will send that later - sorry.
Thanks,
David
On 12/04/2019 5:51 pm, Patrick Zhang OS wrote:
Moved this to core-libs-dev for review, thanks.
Dropped and bcc'ed jdk-dev and jdk-updates-dev.
Regards
Patrick
-----Original Message-----
From: David Holmes <david.hol...@oracle.com>
Sent: Friday, April 12, 2019 3:43 PM
To: Patrick Zhang OS <patr...@os.amperecomputing.com>;
jdk-...@openjdk.java.net
Cc: jdk-updates-...@openjdk.java.net
Subject: Re: RFR: 8222334: java -Xss0 triggers StackOverflowError
Hi Patrick,
Please takes this to core-libs-dev for review.
Thanks,
David
On 12/04/2019 5:24 pm, Patrick Zhang OS wrote:
Hi,
Please review this patch.
The problem is that the launcher does a check on the input -Xss and
ensure it >=64K for the initial thread, while vm has another
function to determine whether the input stack size is big enough to
future threads, such as cgc_thread, vm_thread, java_thead etc.
However if -Xss0, the initial thread is created with stack size
64K, while others use hotspot/system default sizes, which would
trigger StackOverflowError. We could either fine tune the threshold
64K to be a bigger one, or have the initial thread created with
system defaults that may be what the user expects. This patch
chooses the second solution, to avoid potential side-effect of the first.
This can be reproduced with 10, 11, 12 too, so I cc'ed
jdk-updates-dev here.
More details please refer to the ticket.
JBS: https://bugs.openjdk.java.net/browse/JDK-8222334
Webrev: http://cr.openjdk.java.net/~qpzhang/8222334/webrev.01/
Thanks for David's comments in Jira.
Regards
Patrick
