Skip to content

Stack size error message missing important setting #18472

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
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

@goldwaving goldwaving commented Jan 5, 2023

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

goldwaving commented Jan 6, 2023

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

goldwaving commented Jan 11, 2023

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