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

Update thread id validation returned by __wasi_thread_spawn #435

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

loganek
Copy link
Contributor

@loganek loganek commented Aug 30, 2023

According to the documentation: https://github.com/WebAssembly/wasi-threads#design-choice-thread-ids, TID should be in the range <1, 0x1FFFFFFF>

* (`>= 0`) or an error code (`< 0`). As in the unmodified version, all
* (`<1, 0x1FFFFFFF>`) or an error code (`< 0`). Please note that `0` is
* reserved for compatibility reasons and must not be returned by the runtime
* (if that happens, EAGAIN is returned). As in the unmodified version, all
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why EAGAIN?
an assertion is more appropriate.

Copy link
Collaborator

@abrown abrown Sep 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it might be better to crash to force people to go fix the engine. But otherwise this makes sense to me!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough, updated.

@loganek loganek force-pushed the loganek/update-tid-validation branch from 0103e28 to d52365b Compare September 17, 2023 19:48
@loganek loganek force-pushed the loganek/update-tid-validation branch from d52365b to 78e3f09 Compare September 17, 2023 19:49
@abrown abrown merged commit ce2f157 into WebAssembly:main Sep 18, 2023
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 this pull request may close these issues.

3 participants