JDK-8176768 : hotspot ignores PTHREAD_STACK_MIN when creating new threads
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 9,10
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2017-03-14
  • Updated: 2017-08-25
  • Resolved: 2017-03-23
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10
10 b21Fixed
Related Reports
Relates :  
JDK-8175342 - assert(InstanceKlass::cast(k)->is_initialized()) failed: need to increase java_thread_min_stack_allowed calculation
Description
Hotspot creates new threads with the stack size specified by -Xss. It uses pthread_attr_setstacksize() to do that. On xgene systems it's been observed that the call to pthread_attr_setstacksize() is failing with EINVAL when small stack sizes are specified, so we end up with the default 8mb stack size. This happens when the stack size is less than PTHREAD_STACK_MIN. On xgene we allow stack sizes as small as 72k (based on a computation of the minimum size needed), so basically any -Xss stack size of 72k to 124k will produce this result. This means users trying to minimize stack sizes might actually end up doing the opposite when choosing these smaller sizes.

This fix should be simple. When we compute _java_thread_min_stack_allowed, we should always round up to PTHREAD_STACK_MIN. This will causes -Xss stack sizes less than 128k to produce an error message saying the minimum allowed is 128k.
Comments
Another bug to be fixed with this CR is the handling of the jlong thread size passed in from the Thread() constructor. It is converted to a size_t, which is 32-bit unsigned on 32-bit systems. This results in truncating the 64-bit size, possibly resulting in a very small size. The appropriate thing to do is set the size to max allowed in 32-bits.
21-03-2017
POSIX also has a loose requirement that the stack size be OS page aligned. On our supported platforms, only Mac OS X seems to enforce this requirement. Currently on Mac OS X the pthread_attr_setstacksize() calls are silently failing if you set the stack size to a value that is not page aligned. This affects stacks sizes specified when using new Thread() from java app, the VMThreadStackSize argument, and the CompilerThreadStackSize argument. -Xss and ThreadStackSize are not affected because the value passed is already rounded up to the os page size. I'm adding an assert to os::_create_thread() to validate the pthread_attr_setstacksize() result, and will also make sure we always align these stack sizes up to the os page size.
18-03-2017
Given we're going to have special case solaris anyway maybe just use thr_stack_min() size_t stack_min = SOLARIS_ONLY(thr_stack_min()) NOT_SOLARIS(PTHREAD_STACK_MIN) ? Otherwise you just need to set _POSIX_C_SOURCE before the include of <limits.h>. Both ways are messy.
15-03-2017
I guess the following is the relevant part: "Applications that are intended to be conforming POSIX.1 applications must define the feature test macros specified by the standard before including any headers. For the standards listed below, applications must define the feature test macros listed." So for Solaris you need to #define _POSIX_C_SOURCE to the proper value. Do you think then I should take that approach for this bug fix so PTHREAD_STACK_MIN can be used? Basically do what I saw in the one JDK example: #ifndef _POSIX_C_SOURCE #define _POSIX_C_SOURCE 199509L #endif
15-03-2017
Much to my surprise it seems we do not set, nor is there any default for _POSIX_C_SOURCE. Despite the fact that different APIs came in with different POSIX versions (eg 199506L == POSIX.1c-1995 compilation (POSIX Threads)!) it seems that the bulk of the API's, in particular in pthreads.h, are defined unconditionally. Hence we have by sheer chance/luck/fluke gotten by without setting anything!
15-03-2017
For reference: http://docs.oracle.com/cd/E19253-01/816-5175/standards-5/index.html
15-03-2017
The default compilation environment should be set up by the compiler. I need to check the SS docs to see what that is. From memory we were always compiling for XOPEN/SuSV2 (or V3) which should include all the POSIX stuff. Note above that some of those checks are checking for different POSIX versions, but it is a mess for sure.
15-03-2017
Ok. I still think the OS min stack size support should be moved to os::Posix::set_minimum_stack_sizes() and apply to the overall stack size, not just the size before we add red, yellow, and shadow zones. I don't know what to say about _POSIX_C_SOURCE. I did a grep and didn't find it referenced in any of the hotspot sources or makefiles. I assume you expect gcc to #define it. I did find a few references in jdk sources: #if (_POSIX_C_SOURCE >= 200809L) || defined(__solaris__) To me this one would imply that _POSIX_C_SOURCE is not #define'd for solaris. #if defined(LINUX) && (defined(_GNU_SOURCE) || \ (defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE < 200112L \ && defined(_XOPEN_SOURCE) && _XOPEN_SOURCE < 600)) And this one is only for Linux. #ifndef _POSIX_C_SOURCE #define _POSIX_C_SOURCE 199309L #endif This one might be trying to work around the #define missing for Solaris. Although there are no other references in to _POSIX_C_SOURCE in the .cc file it is in, it is done before system header files are included.
15-03-2017
Before 8140520 we already had: Solaris::min_stack_allowed = MAX2(thr_min_stack(), Solaris::min_stack_allowed); and it was used for various types of threads. All 8140520 did was use different values for different types of threads - but all still applied the thr_min_stack() limit. I'm very surprised that PTHREAD_STACK_MIN is not available as we should certainly be compiling with _POSIX_C_SOURCE >= 199506L implicitly! That defines basic POSIX support as included with UNIX98/SuSV2
15-03-2017
For the fix I made sure the 3 min stack sizes we setup in os::Posix::set_minimum_stack_sizes() are all set to at least PTHREAD_STACK_MIN. This seems to be working, except on Solaris it won't build. For some reason the #define for PTHREAD_STACK_MIN is protected by "#if defined(__EXTENSIONS__) || (_POSIX_C_SOURCE >= 199506L)". I'm not sure why, but am not inclined to enable either of these to get this working. I noticed in solaris in os::init() we already make sure the stack meets the OS minimum requirements: // Constant minimum stack size allowed. It must be at least // the minimum of what the OS supports (thr_min_stack()), and // enough to allow the thread to get to user bytecode execution. Posix::_compiler_thread_min_stack_allowed = MAX2(thr_min_stack(), Posix::_compiler_thread_min_stack_allowed); Posix::_java_thread_min_stack_allowed = MAX2(thr_min_stack(), Posix::_java_thread_min_stack_allowed); Posix::_vm_internal_thread_min_stack_allowed = MAX2(thr_min_stack(), Posix::_vm_internal_thread_min_stack_allowed); There's good and bad in this. It shows that thr_min_stack() is an alternative to using PTHREAD_STACK_MIN on solaris. What's bad about this code is that this is all done before the code in os::Posix::set_minimum_stack_sizes(). All these sizes still need to have red, yellow, and shadow zones added to them, so they eventually will be much bigger, and likely setting them to be at least thr_min_stack() here is not even needed. So that means when the user is trying to keep the stack as small as possible, it's made unnecessarily big here. I think the code above should be removed, and let os::Posix::set_minimum_stack_sizes() be where we make sure the stacks are meeting the minimum OS requirements. We can add an API an os::Posix API to return the minimum allowed stack size in an os independent way. So we can use thr_min_stack() for solaris, and PTHREAD_STACK_MIN for everything else. Or we can just #ifdef the code for Solaris. The above code was added by JDK-8140520. I'm not sure why. If there were -Xss values that were resulting in crashes or asserts, then the initial _java_thread_min_stack_allowed should have been made bigger, not just rounding up to thr_min_stack().
15-03-2017