Re: RFR: 8222334: java -Xss0 triggers StackOverflowError
Thanks Patrick! Changes pushed. David On 16/04/2019 8:44 pm, Patrick Zhang OS wrote: Done. http://cr.openjdk.java.net/~qpzhang/8222334/webrev.05 Regards Patrick -Original Message- From: David Holmes Sent: Tuesday, April 16, 2019 6:34 PM To: Patrick Zhang OS Cc: core-libs-dev Subject: Re: RFR: 8222334: java -Xss0 triggers StackOverflowError Hi Patrick, On 16/04/2019 7:42 pm, Patrick Zhang OS wrote: Hi David, Please see my updates, the two '0' size test cases. I have run them with jtreg on jdk13 + linux + x86/aarch64 systems respectively, all passed. http://cr.openjdk.java.net/~qpzhang/8222334/webrev.04 Thanks. Please update copyright years. Also instead of this comment: It can verify the issue fixed in 8222334. Just add 8222334 to the @bug line. Thanks, David Regards Patrick -Original Message- From: core-libs-dev On Behalf Of Patrick Zhang OS Sent: Tuesday, April 16, 2019 4:23 PM To: David Holmes Cc: core-libs-dev Subject: RE: RFR: 8222334: java -Xss0 triggers StackOverflowError Sure I will add this, and fix the intention mentioned by Alan. Regards Patrick -Original Message- From: David Holmes Sent: Tuesday, April 16, 2019 4:17 PM To: Patrick Zhang OS Cc: Alan Bateman ; core-libs-dev Subject: Re: RFR: 8222334: java -Xss0 triggers StackOverflowError Patrick, Sorry should have picked up on this earlier. Can you please update the following two tests to add a test for '0' as appropriate: ./jdk/tools/launcher/TooSmallStackSize.java ./hotspot/jtreg/runtime/Thread/TooSmallStackSize.java Thanks, David On 16/04/2019 5:47 pm, David Holmes wrote: On 16/04/2019 5:40 pm, Alan Bateman wrote: On 15/04/2019 08:48, David Holmes wrote: On 15/04/2019 5:34 pm, Patrick Zhang OS wrote: Removed it. http://cr.openjdk.java.net/~qpzhang/8222334/webrev.03/jdk.changese t 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 ;-) ) This looks okay to me too, I think we should fix the intention in ContinueInNewThread while we are there so it matches the rest of the file. Thanks Alan! I'll fix the indent before pushing. David - -Alan
RE: RFR: 8222334: java -Xss0 triggers StackOverflowError
Done. http://cr.openjdk.java.net/~qpzhang/8222334/webrev.05 Regards Patrick -Original Message- From: David Holmes Sent: Tuesday, April 16, 2019 6:34 PM To: Patrick Zhang OS Cc: core-libs-dev Subject: Re: RFR: 8222334: java -Xss0 triggers StackOverflowError Hi Patrick, On 16/04/2019 7:42 pm, Patrick Zhang OS wrote: > Hi David, > Please see my updates, the two '0' size test cases. I have run them with > jtreg on jdk13 + linux + x86/aarch64 systems respectively, all passed. > http://cr.openjdk.java.net/~qpzhang/8222334/webrev.04 Thanks. Please update copyright years. Also instead of this comment: It can verify the issue fixed in 8222334. Just add 8222334 to the @bug line. Thanks, David > Regards > Patrick > > -Original Message- > From: core-libs-dev On Behalf > Of Patrick Zhang OS > Sent: Tuesday, April 16, 2019 4:23 PM > To: David Holmes > Cc: core-libs-dev > Subject: RE: RFR: 8222334: java -Xss0 triggers StackOverflowError > > Sure I will add this, and fix the intention mentioned by Alan. > > Regards > Patrick > > -Original Message- > From: David Holmes > Sent: Tuesday, April 16, 2019 4:17 PM > To: Patrick Zhang OS > Cc: Alan Bateman ; core-libs-dev > > Subject: Re: RFR: 8222334: java -Xss0 triggers StackOverflowError > > Patrick, > > Sorry should have picked up on this earlier. Can you please update the > following two tests to add a test for '0' as appropriate: > > ./jdk/tools/launcher/TooSmallStackSize.java > ./hotspot/jtreg/runtime/Thread/TooSmallStackSize.java > > Thanks, > David > > On 16/04/2019 5:47 pm, David Holmes wrote: >> On 16/04/2019 5:40 pm, Alan Bateman wrote: >>> On 15/04/2019 08:48, David Holmes wrote: >>>> On 15/04/2019 5:34 pm, Patrick Zhang OS wrote: >>>>> Removed it. >>>>> http://cr.openjdk.java.net/~qpzhang/8222334/webrev.03/jdk.changese >>>>> t >>>>> >>>>> 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 ;-) ) >>> This looks okay to me too, I think we should fix the intention in >>> ContinueInNewThread while we are there so it matches the rest of the >>> file. >> >> Thanks Alan! I'll fix the indent before pushing. >> >> David >> - >> >>> -Alan
Re: RFR: 8222334: java -Xss0 triggers StackOverflowError
Hi Patrick, On 16/04/2019 7:42 pm, Patrick Zhang OS wrote: Hi David, Please see my updates, the two '0' size test cases. I have run them with jtreg on jdk13 + linux + x86/aarch64 systems respectively, all passed. http://cr.openjdk.java.net/~qpzhang/8222334/webrev.04 Thanks. Please update copyright years. Also instead of this comment: It can verify the issue fixed in 8222334. Just add 8222334 to the @bug line. Thanks, David Regards Patrick -Original Message- From: core-libs-dev On Behalf Of Patrick Zhang OS Sent: Tuesday, April 16, 2019 4:23 PM To: David Holmes Cc: core-libs-dev Subject: RE: RFR: 8222334: java -Xss0 triggers StackOverflowError Sure I will add this, and fix the intention mentioned by Alan. Regards Patrick -Original Message- From: David Holmes Sent: Tuesday, April 16, 2019 4:17 PM To: Patrick Zhang OS Cc: Alan Bateman ; core-libs-dev Subject: Re: RFR: 8222334: java -Xss0 triggers StackOverflowError Patrick, Sorry should have picked up on this earlier. Can you please update the following two tests to add a test for '0' as appropriate: ./jdk/tools/launcher/TooSmallStackSize.java ./hotspot/jtreg/runtime/Thread/TooSmallStackSize.java Thanks, David On 16/04/2019 5:47 pm, David Holmes wrote: On 16/04/2019 5:40 pm, Alan Bateman wrote: On 15/04/2019 08:48, David Holmes wrote: 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 ;-) ) This looks okay to me too, I think we should fix the intention in ContinueInNewThread while we are there so it matches the rest of the file. Thanks Alan! I'll fix the indent before pushing. David - -Alan
RE: RFR: 8222334: java -Xss0 triggers StackOverflowError
Hi David, Please see my updates, the two '0' size test cases. I have run them with jtreg on jdk13 + linux + x86/aarch64 systems respectively, all passed. http://cr.openjdk.java.net/~qpzhang/8222334/webrev.04 Regards Patrick -Original Message- From: core-libs-dev On Behalf Of Patrick Zhang OS Sent: Tuesday, April 16, 2019 4:23 PM To: David Holmes Cc: core-libs-dev Subject: RE: RFR: 8222334: java -Xss0 triggers StackOverflowError Sure I will add this, and fix the intention mentioned by Alan. Regards Patrick -Original Message- From: David Holmes Sent: Tuesday, April 16, 2019 4:17 PM To: Patrick Zhang OS Cc: Alan Bateman ; core-libs-dev Subject: Re: RFR: 8222334: java -Xss0 triggers StackOverflowError Patrick, Sorry should have picked up on this earlier. Can you please update the following two tests to add a test for '0' as appropriate: ./jdk/tools/launcher/TooSmallStackSize.java ./hotspot/jtreg/runtime/Thread/TooSmallStackSize.java Thanks, David On 16/04/2019 5:47 pm, David Holmes wrote: > On 16/04/2019 5:40 pm, Alan Bateman wrote: >> On 15/04/2019 08:48, David Holmes wrote: >>> 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 ;-) ) >> This looks okay to me too, I think we should fix the intention in >> ContinueInNewThread while we are there so it matches the rest of the >> file. > > Thanks Alan! I'll fix the indent before pushing. > > David > - > >> -Alan
RE: RFR: 8222334: java -Xss0 triggers StackOverflowError
Sure I will add this, and fix the intention mentioned by Alan. Regards Patrick -Original Message- From: David Holmes Sent: Tuesday, April 16, 2019 4:17 PM To: Patrick Zhang OS Cc: Alan Bateman ; core-libs-dev Subject: Re: RFR: 8222334: java -Xss0 triggers StackOverflowError Patrick, Sorry should have picked up on this earlier. Can you please update the following two tests to add a test for '0' as appropriate: ./jdk/tools/launcher/TooSmallStackSize.java ./hotspot/jtreg/runtime/Thread/TooSmallStackSize.java Thanks, David On 16/04/2019 5:47 pm, David Holmes wrote: > On 16/04/2019 5:40 pm, Alan Bateman wrote: >> On 15/04/2019 08:48, David Holmes wrote: >>> 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 ;-) ) >> This looks okay to me too, I think we should fix the intention in >> ContinueInNewThread while we are there so it matches the rest of the >> file. > > Thanks Alan! I'll fix the indent before pushing. > > David > - > >> -Alan
Re: RFR: 8222334: java -Xss0 triggers StackOverflowError
Patrick, Sorry should have picked up on this earlier. Can you please update the following two tests to add a test for '0' as appropriate: ./jdk/tools/launcher/TooSmallStackSize.java ./hotspot/jtreg/runtime/Thread/TooSmallStackSize.java Thanks, David On 16/04/2019 5:47 pm, David Holmes wrote: On 16/04/2019 5:40 pm, Alan Bateman wrote: On 15/04/2019 08:48, David Holmes wrote: 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 ;-) ) This looks okay to me too, I think we should fix the intention in ContinueInNewThread while we are there so it matches the rest of the file. Thanks Alan! I'll fix the indent before pushing. David - -Alan
Re: RFR: 8222334: java -Xss0 triggers StackOverflowError
On 16/04/2019 5:40 pm, Alan Bateman wrote: On 15/04/2019 08:48, David Holmes wrote: 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 ;-) ) This looks okay to me too, I think we should fix the intention in ContinueInNewThread while we are there so it matches the rest of the file. Thanks Alan! I'll fix the indent before pushing. David - -Alan
Re: RFR: 8222334: java -Xss0 triggers StackOverflowError
On 15/04/2019 08:48, David Holmes wrote: 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 ;-) ) This looks okay to me too, I think we should fix the intention in ContinueInNewThread while we are there so it matches the rest of the file. -Alan
Re: RFR: 8222334: java -Xss0 triggers StackOverflowError
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 Sent: Monday, April 15, 2019 2:33 PM To: Patrick Zhang OS ; core-libs-dev 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 Sent: Monday, April 15, 2019 6:55 AM To: Patrick Zhang OS ; core-libs-dev 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 Sent: Friday, April 12, 2019 3:43 PM To: Patrick Zhang OS ; 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 thre
RE: RFR: 8222334: java -Xss0 triggers StackOverflowError
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 Sent: Monday, April 15, 2019 2:33 PM To: Patrick Zhang OS ; core-libs-dev 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 > Sent: Monday, April 15, 2019 6:55 AM > To: Patrick Zhang OS ; core-libs-dev > > 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. >>> >>> D
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 Sent: Monday, April 15, 2019 6:55 AM To: Patrick Zhang OS ; core-libs-dev 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 Sent: Friday, April 12, 2019 3:43 PM To: Patrick Zhang OS ; 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
RE: RFR: 8222334: java -Xss0 triggers StackOverflowError
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 Sent: Monday, April 15, 2019 6:55 AM To: Patrick Zhang OS ; core-libs-dev 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 >> Sent: Friday, April 12, 2019 3:43 PM >> To: Patrick Zhang OS ; >> 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
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 Sent: Friday, April 12, 2019 3:43 PM To: Patrick Zhang OS ; 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
Re: RFR: 8222334: java -Xss0 triggers StackOverflowError
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 Sent: Friday, April 12, 2019 3:43 PM To: Patrick Zhang OS ; 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
RE: RFR: 8222334: java -Xss0 triggers StackOverflowError
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 Sent: Friday, April 12, 2019 3:43 PM To: Patrick Zhang OS ; 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 >
