Hi David, Many thanks, I integrated your updates into the new patch.
> 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 >>>
