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