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

Assertion error: when pthread returns an error, this library crashes the entire program #115

Closed
lovasoa opened this issue Feb 18, 2025 · 6 comments · Fixed by #116
Closed

Comments

@lovasoa
Copy link
Contributor

lovasoa commented Feb 18, 2025

Hello !
I work on SQLPage.
Stacker was recently introduced as a transitive dependency (through sqlparser).
I just received a bug report from an user that involves stacker: sqlpage/SQLPage#814

The problem seems to be in https://github.com/rust-lang/stacker/blob/04d334ca6ad8ef942fef80e173c6e34bbb905a6a/src/lib.rs#L418C30-L418C51 , where pthread_attr_getstack returns 2 (ENOENT).

I am not sure how to proceed, but wanted to report the issue here. Do you have an idea what may be going on and how to fix it ?

@lovasoa
Copy link
Contributor Author

lovasoa commented Feb 18, 2025

Ok, I can reproduce it locally now !

sudo mount -t tmpfs none /proc

./sqlpage.bin

WIth sqlpage.bin from https://github.com/sqlpage/SQLPage/releases/tag/v0.33.0

@lovasoa lovasoa changed the title Assertion error: pthread_attr_getstack returns ENOENT ?! Assertion error: pthread_attr_getstack returns ENOENT when /proc is not mounted Feb 18, 2025
@nagisa
Copy link
Member

nagisa commented Feb 18, 2025

yeah, I don't think glibc targets where procfs isn't mounted at /proc is something that's going to be supported like ever. But if you think it should be, that's a report for glibc.

@nagisa
Copy link
Member

nagisa commented Feb 18, 2025

stacker could conceptually try to have some fallback like it does for systems that don't have a way to query stack size, but I don't think this corner case is particularly interesting for stacker to support… at that point you'd be better off using psm and creating your stacks yourself. Or perhaps corosensei.

@lovasoa
Copy link
Contributor Author

lovasoa commented Feb 18, 2025

Would it be hard to implement proper error handling? Crashing the entire program on startup, when the stack doesn't even need to grow, is not ideal...

I ended up with this crate as a sub dependency in a large program that worked without issues in this environment before...

lovasoa added a commit to lovasoa/stacker that referenced this issue Feb 18, 2025
@lovasoa
Copy link
Contributor Author

lovasoa commented Feb 18, 2025

@nagisa , I made a proposition to add proper error handling in https://github.com/rust-lang/stacker/pull/116/files

Let me know what you think !

@lovasoa lovasoa changed the title Assertion error: pthread_attr_getstack returns ENOENT when /proc is not mounted Assertion error: when pthread returns an error, this library crashes the entire program Feb 18, 2025
nagisa pushed a commit that referenced this issue Feb 21, 2025
* improve error handling

see #115

See sqlpage/SQLPage#814

* improve error message in destroy_pthread_attr

* remove code duplication

* cleanup code and add comments

* Make error handling code easier to read

* update formatting

https://github.com/rust-lang/stacker/pull/116/files#r1961535064

* put the beautiful unicode apostrophe back

https://github.com/rust-lang/stacker/pull/116/files#r1961536979

* use one file per guess_os_stack_limit implementation

* fix error handling on openbsd too

* beautiful unicode apostrophe

* format

* fix windows build

* remove unused unsafe

* fix error handling on windows

VirtualQuery is a faillible function that was treated as infaillible
https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualquery

* SetThreadStackGuarantee can also error on windows

* simplify pthread error handling
@lovasoa
Copy link
Contributor Author

lovasoa commented Feb 21, 2025

That you very much for merging ! Could you publish a new version so that I can update my dependencies ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants