-
Notifications
You must be signed in to change notification settings - Fork 206
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
__init_tp: Initialize TID to non-zero value #360
Conversation
* - __lockfile relies on the fact the most significant two bits | ||
* of TIDs are 0. | ||
*/ | ||
td->tid = 0x3fffffff; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initialize the TID to a large value, which is hopefully
less likely to conflict with host-allocated TIDs
So, that assumption is correct only if the host assigns the ID incrementally, which might not be the case (host can decide the implementation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initialize the TID to a large value, which is hopefully
less likely to conflict with host-allocated TIDsSo, that assumption is correct only if the host assigns the ID incrementally, which might not be the case (host can decide the implementation).
right. this is a kludge at best.
i think a real fix involves spec changes. see WebAssembly/wasi-threads#15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest let's wait with this PR then until the WebAssembly/wasi-threads#15 is resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can update the PR based on the discussion in WebAssembly/wasi-threads#15 @yamt ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can update the PR based on the discussion in WebAssembly/wasi-threads#15 @yamt ?
done
This fixes TID-based locking used within libc. Also, initialize detach_state. cf. WebAssembly/wasi-threads#16
i updated the comment after WebAssembly/wasi-threads#16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine; I would want to link to the wasi-threads heading where this is discussed, but that's personal preference.
This fixes TID-based locking used within libc. Also, initialize detach_state. cf. WebAssembly/wasi-threads#16
__init_tp: Initialize TID to non-zero value
This fixes TID-based locking used within libc.
Also, initialize detach_state.
cf. WebAssembly/wasi-threads#16