RFR(L) 8140520 segfault on solaris-amd64 with "-XX:VMThreadStackSize=1" option

[Daniel D. Daugherty]

On 9/8/16 10:43 PM, David Holmes wrote:
> Hi Dan,
>
> On 9/09/2016 11:05 AM, Daniel D. Daugherty wrote:
>>> Future Work:
>>>
>>> - Switch TooSmallStackSize.java from local TestHelper.java to the newer
>>>   test framework
>>>
>>>   - TestHelper.java is a copy of the launcher's TestHelper.java since
>>>     that was the easiest way to bring up the new test. The dependencies
>>>     in TooSmallStackSize.java on TestHelper.java need to be identified
>>>     and replaced with newer test library equivalents if possible.
>>
>> hotspot/test/runtime/Thread/TooSmallStackSize.java has been rewritten
>> to use the newer test framework and
>> hotspot/test/runtime/Thread/TestHelper.java
>> is no longer included in the path.
>>
>> Webrev URL:
>> http://cr.openjdk.java.net/~dcubed/8140520-webrev/4-jdk9-hs-open/
>>
>> Other than timestamps, only
>> hotspot/test/runtime/Thread/TooSmallStackSize.java
>> is changed in this round.
>>
>> I need just a couple of reviews of the new test.
>
> Seems okay. Only comment on the new and existing test is that here:
>
> 86         System.out.println("FAILED: Could not get the stack size 
> from the output");
> 87         throw new RuntimeException("test fails");
>
> we really want to see the output that didn't have what we expected. 
> Will it shown in the log anyway?

I'll induce a failure in both tests and see what we get.

Dan


>
> Thanks,
> David
>
>
>> I added some of the test output to this comment in the bug report:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8140520?focusedCommentId=13999090&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13999090 
>>
>>
>>
>> so the new test passes on all platforms.
>>
>> Dan
>>
>>
>>
>> On 9/2/16 12:28 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> I am shepherding the fix for the following bug for Ron Durbin:
>>>
>>>     JDK-8140520 segfault on solaris-amd64 with
>>> "-XX:VMThreadStackSize=1" option
>>>     https://bugs.openjdk.java.net/browse/JDK-8140520
>>>
>>> Almost all of the product code is Ron's, all of the test code is 
>>> mine and
>>> all of the product code refactoring is Coleen's. I will be handling the
>>> responses to all code review comments and suggestions. Thanks to Jerry
>>> for
>>> reviewing earlier versions of Ron's work on this bug. Thanks to 
>>> David H.
>>> and Coleen for reviewing versions after I started shepherding.
>>>
>>> The primary focus of this change is to decouple the various thread 
>>> stack
>>> sizes from each other. In particular, decouple CompilerThreadStackSize
>>> from VMThreadStackSize to avoid use of VMThreadStackSize defaults for
>>> CompilerThreadStackSize.
>>>
>>> There are almost no Win* changes here because the Win* thread stack 
>>> size
>>> implementation differs significantly from the *NIX implementation. I 
>>> have
>>> a first pass implementation for the Win* changes, but Win* stack pages
>>> don't behave like I expect so I will split Win* off into another bug.
>>>
>>> The AIX and PPC parts of the fix are "best guesses" because we don't 
>>> have
>>> a way to build and test AIX and/or PPC in Oracle. I'll need help 
>>> from the
>>> usual AIX and PPC folks for checking out those changes.
>>>
>>> Overview of the changes:
>>>
>>> - The CompilerThreadStackSize default has been changed from zero to
>>>   match the default VMThreadStackSize on each platform; see the various
>>>   globals_*.hpp files.
>>>
>>> - There are now three minimum stack sizes: 
>>> java_thread_min_stack_allowed
>>>   compiler_thread_min_stack_allowed, and
>>> vm_internal_thread_min_stack_allowed.
>>>   In all cases except for Solaris X64, the two new minimums are
>>> initialized
>>>   to same value as the original min_stack_allowed; see the various
>>>   */os_cpu/*/os_*.cpp files.
>>>
>>>   Key change: On Solaris X64 the new compiler_thread_min_stack_allowed
>>>   value is 394K instead of the old min_stack_allowed value of 224K.
>>>
>>> - os::create_thread()'s stack size parameter has been clarified to be
>>>   the "requested stack size". The stack size adjustment logic has been
>>>   updated to support the three minimum stack sizes.
>>>
>>>   Key change: the VMThreadStackSize default is no longer used as the
>>>   CompilerThreadStackSize default.
>>>
>>> - os::init_2()'s minimum stack size setting and checking logic has
>>>   been updated to support the three minimum stack sizes.
>>>
>>>   Key change: CompilerThreadStackSize and VMThreadStackSize are no 
>>> longer
>>>   silently rounded up when their specified value is too small. This
>>> matches
>>>   the behavior for both '-Xss<n_bytes>' and
>>> '-XX:ThreadStackSize=<k_bytes>'.
>>>
>>>   Note: The recent fix for the following bug:
>>>
>>>     8159335: Fix problems with stack overflow handling.
>>>
>>>   updated the existing minimum stack size calculations to ensure that
>>> they
>>>   are aligned with os::vm_page_size(). I've updated Ron's solution for
>>>   this bug to include the new alignment work with the two new minimum
>>>   stack sizes.
>>>
>>> Bugs fixed:
>>>
>>> - The Solaris X64 crash when "-XX:VMThreadStackSize=1" is specified
>>>   without specifying "-XX:CompilerThreadStackSize=<k_bytes>" is fixed.
>>> - Newly defined ThreadType values are explicitly handled as VM internal
>>>   threads instead of skating through the case statements.
>>> - On platforms that use default_stack_size(), the bug where a default
>>>   value returned for a compiler_thread was ignored is fixed.
>>>
>>> Changes in behavior:
>>>
>>> - Use of -XX:VMThreadStackSize=<k_foo> will no longer affect the value
>>>   of CompilerThreadStackSize when -XX:CompilerThreadStackSize=<bar>
>>>   is not specified.
>>> - When neither -XX:VMThreadStackSize=<foo> or
>>> -XX:CompilerThreadStackSize=<bar>
>>>   are specified on the command line, then CompilerThreadStackSize will
>>> use
>>>   a distinct default value than VMThreadStackSize.
>>> - Use of a too small -XX:CompilerThreadStackSize=<bar> value or
>>>   a -XX:VMThreadStackSize=<foo> will generate an error message
>>>   instead of silently rounding up.
>>>
>>> Refactoring changes:
>>>
>>> - os::Posix::get_initial_stack_size() and
>>> os::Posix::set_minimum_stack_sizes()
>>>   created as a place to land common *NIX code; once we figure out how
>>> to add
>>>   support for the three thread stack sizes to Win*, this code will
>>> move again.
>>> - default_stack_size() added to all platforms.
>>> - default_guard_size() removed from platforms that were not using it;
>>> only
>>>   Linux uses it.
>>>
>>>
>>> Webrev URL:
>>> http://cr.openjdk.java.net/~dcubed/8140520-webrev/3-jdk9-hs-open/
>>>
>>>
>>> Testing:
>>>
>>> - There is a temporary change in hotspot/test/TEST.groups that allows
>>> runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java to
>>>   run during JPRT jobs. That test is normally excluded from the
>>>   hotspot_fast_runtime group in JPRT jobs.
>>> - I've copied and modified the TooSmallStackSize.java test from the
>>>   launcher's tests in the jdk repo. The jdk repo version exercises the
>>>   '-Xss<n_bytes>' option. The hotspot repo version exercises the
>>>   '-XX:ThreadStackSize=<k_bytes>',
>>> '-XX:CompilerThreadStackSize=<k_bytes>',
>>>   and '-XX:VMThreadStackSize=<k_bytes>' options.
>>> - RBT JDK9-hs nightly results were generated and no regressions were
>>>   observed; another round was started after the latest resync, but has
>>>   not yet finished
>>>
>>>
>>> Future Work:
>>>
>>> - Add {compiler,vm_internal}_thread_min_stack_allowed support to 
>>> Windows
>>>
>>>   - Have the beginnings of an implementation, but stack sizes and the
>>>     underlying pages aren't working as I expect.
>>>
>>> - Switch TooSmallStackSize.java from local TestHelper.java to the newer
>>>   test framework
>>>
>>>   - TestHelper.java is a copy of the launcher's TestHelper.java since
>>>     that was the easiest way to bring up the new test. The dependencies
>>>     in TooSmallStackSize.java on TestHelper.java need to be identified
>>>     and replaced with newer test library equivalents if possible.
>>>
>>> - Determine whether the minimum stack values can be reduced on the
>>>   different platforms.
>>>
>>>   - With a single minimum stack size value, it's possible that the
>>>     minimum has been bumped up over the years for one thread type,
>>>     e.g., Compiler threads, when the other thread types didn't need
>>>     the extra space.
>>>   - Will need to do some spelunking through the change histories
>>>     and then some old fashioned value range testing.
>>>
>>> - There is a block of Solaris specific code in
>>>   os::Posix::set_minimum_stack_sizes(); Coleen filed the following
>>>   bug to remove that code:
>>>
>>>   JDK-8161093 Solaris for >8k pagesize adds extra guard pages
>>>   https://bugs.openjdk.java.net/browse/JDK-8161093
>>>
>>> Thanks, in advance, for any comments, suggestions or questions!
>>>
>>> Dan (, Ron and Coleen)
>>>
>>