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.

Sign up for GitHub

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

Jump to bottom

Stack size error message missing important setting #18472

Closed
wants to merge 1 commit into from
Closed

Stack size error message missing important setting #18472

wants to merge 1 commit into from

Conversation

goldwaving
Copy link
Contributor

Please considering mentioning DEFAULT_PTHREAD_STACK_SIZE alongside STACK_SIZE in the runtime error reporting. DEFAULT_PTHREAD_STACK_SIZE is not well documented. Anyone using pthreads will waste a lot of time trying to figure out why their code is failing (I know I did).

@goldwaving
Stack size error message missing important setting
730a605 
Please considering mentioning DEFAULT_PTHREAD_STACK_SIZE alongside STACK_SIZE in the runtime error reporting.  DEFAULT_PTHREAD_STACK_SIZE is not well documented.  Anyone using pthreads will waste a lot of time trying to figure out why their code is failing (I know I did).
@sbc100
Copy link
Collaborator

sbc100 commented Jan 5, 2023

Ah, sorry about that. When I made that change my indent was to have DEFAULT_PTHREAD_STACK_SIZE default to the same value as STACK_SIZE. I'm not sure how I missed that part of the change when I landed #18191.

The idea is that setting -sSTACK_SIZE should also set -sDEFAULT_PTHREAD_STACK_SIZE so it should not be needed. I'll see if I can make a change that effect now.

@goldwaving
Copy link
Contributor Author

Thanks for the reply. That makes sense.

sbc100 added a commit that referenced this pull request Jan 9, 2023
@sbc100
Set pthread stack size when -sSTACK_SIZE is set
4918f7c 
This was always my intention with #18191 but somehow it got left out.  I
think it makes sense that user to override `STACK_SIZE` also, by
default, override the pthread stack size.  Such users can always
explicitly set the pthread stack size if they don't want this behaviour.

See #18472
@sbc100 sbc100 mentioned this pull request Jan 9, 2023
Merged
sbc100 added a commit that referenced this pull request Jan 9, 2023
@sbc100
Set pthread stack size when -sSTACK_SIZE is set
15c8845 
This was always my intention with #18191 but somehow it got left out.  I
think it makes sense that user to override `STACK_SIZE` also, by
default, override the pthread stack size.  Such users can always
explicitly set the pthread stack size if they don't want this behaviour.

See #18472
sbc100 added a commit that referenced this pull request Jan 9, 2023
@sbc100
Set pthread stack size when -sSTACK_SIZE is set
7042b11 
This was always my intention with #18191 but somehow it got left out.  I
think it makes sense that user to override `STACK_SIZE` also, by
default, override the pthread stack size.  Such users can always
explicitly set the pthread stack size if they don't want this behaviour.

See #18472
sbc100 added a commit that referenced this pull request Jan 9, 2023
@sbc100
Set pthread stack size when -sSTACK_SIZE is set
e04cbae 
This was always my intention with #18191 but somehow it got left out.  I
think it makes sense that user to override `STACK_SIZE` also, by
default, override the pthread stack size.  Such users can always
explicitly set the pthread stack size if they don't want this behaviour.

See #18472
sbc100 added a commit that referenced this pull request Jan 9, 2023
@sbc100
Set pthread stack size when -sSTACK_SIZE is set (#18479)
1d45c23 
This was always my intention with #18191 but somehow it got left out.  I
think it makes sense that user to override `STACK_SIZE` also, by
default, override the pthread stack size.  Such users can always
explicitly set the pthread stack size if they don't want this behaviour.

See #18472
@sbc100
Copy link
Collaborator

sbc100 commented Jan 9, 2023

Hopefully this is fixed in #18479 and we can close this?

@goldwaving
Copy link
Contributor Author

Sounds good.

@sbc100 sbc100 closed this Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@goldwaving @sbc100