Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle users requesting a too-small stack size #11694

Closed
Aatch opened this issue Jan 21, 2014 · 3 comments · Fixed by #11885
Closed

Handle users requesting a too-small stack size #11694

Aatch opened this issue Jan 21, 2014 · 3 comments · Fixed by #11885
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.

Comments

@Aatch
Copy link
Contributor

Aatch commented Jan 21, 2014

pthreads has a minimum stack size of 16KB, if you try to set it to lower than this, pthread_attr_setstacksize will return EINVAL. We don't really handle the errors properly (other than checking the return code), so a user requesting a small stack hits the assert, which isn't very helpful.

There are two options for dealing with this: silently clamp the value to the minimum size for whatever platform we're on, or throw a real error somewhere higher in the callstack with a better error message.

I would just fix it, but I feel I need some input on which choice to make.

@alexcrichton
Copy link
Member

I would expect the best way to handle this would be to silently clamp to a minimum stack size. Throwing an error would start exposing platform-specific functionality as your 4k stacks would work on X but when running on Y everything would start dying.

@ghost
Copy link

ghost commented Jan 21, 2014

I prefer better error message - when I set stack size explicitly I don't expect it to get clamped, also if I want to write platform-agnostic code I shouldn't be setting stack size explicitly - as imo it's already platform specific.

@liigo
Copy link
Contributor

liigo commented Jan 21, 2014

2014/1/21 Alex Crichton [email protected]

I would expect the best way to handle this would be to silently clamp to a
minimum stack size. Throwing an error would start exposing
platform-specific functionality as your 4k stacks would work on X but when
running on Y everything would start dying.

+1, for not exposing platform-specific implementation.

bors added a commit that referenced this issue Feb 1, 2014
EINVAL means that the requested stack size is either not a multiple
of the system page size or that it's smaller than PTHREAD_STACK_MIN.
Figure out what the case is, fix it up and retry.  If it still fails,
give up, like before.

Suggestions for future improvements:

  * don't fail!() but instead signal a condition, or
  * silently ignore the error and use a default sized stack.

Fixes #11694.

The first two commits put the framework in place, the third one contains the meat.
@bors bors closed this as completed in b02b5cd Feb 1, 2014
flip1995 pushed a commit to flip1995/rust that referenced this issue Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants