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

threads: enable more of pthread_mutexattr_setprotocol #383

Closed
wants to merge 1 commit into from
Closed

threads: enable more of pthread_mutexattr_setprotocol #383

wants to merge 1 commit into from

Conversation

abrown
Copy link
Collaborator

@abrown abrown commented Jan 11, 2023

While using wasi-threads with wasi-libc, I found a use of this branch of the code. Adding in the WASI version of futex_wait seems to fix things.

@abrown
threads: enable more of pthread_mutexattr_setprotocol
158e29e 
While using wasi-threads with wasi-libc, I found a use of this branch of
the code. Adding in the WASI version of `futex_wait` seems to fix
things.
yamt
yamt suggested changes Jan 12, 2023
View reviewed changes
Copy link
Contributor

@yamt yamt left a comment

Choose a reason for hiding this comment

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

it doesn't make sense as wasm doesn't have priority or its inheritance.

@abrown abrown mentioned this pull request Jan 26, 2023
Merged
@yamt
Copy link
Contributor

yamt commented Jan 27, 2023

when i use PI, i need it actually work.
i feel it's more user friendly to raise an error than silently ignoring it.

besides that, this implementation looks inappropriate even as a dummy implementation:

  • __wasilibc_futex_wait doesn't implement FUTEX_LOCK_PI
  • i believe the original FUTEX_LOCK_PI call here is to detect the existence of kernel functionality

@abrown
Copy link
Collaborator Author

abrown commented Jan 27, 2023

Sure, let's close this.

@abrown abrown closed this Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yamt yamt yamt requested changes

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
@abrown @yamt