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

feat(wasm-bindgen): deadlock detector support #379

Merged
merged 1 commit into from
Apr 30, 2023
Merged

feat(wasm-bindgen): deadlock detector support #379

merged 1 commit into from
Apr 30, 2023

Conversation

samdenty
Copy link
Contributor

This adds deadlock_detector support for the WASM target.

Instead of thread_id I added a conditional worker_name field

@samdenty
Copy link
Contributor Author

pinging @Amanieu in case you didn't see it 🙂

Amanieu
Amanieu reviewed Apr 30, 2023
View reviewed changes
core/Cargo.toml Outdated Show resolved Hide resolved
@Amanieu
Copy link
Owner

Amanieu commented Apr 30, 2023

LGTM, can you squash the commits?

@samdenty samdenty changed the title feat(wasm): deadlock detector support feat(wasm-bindgen): deadlock detector support Apr 30, 2023
@samdenty
Copy link
Contributor Author

@Amanieu squashed 🙏

@Amanieu Amanieu merged commit de2a402 into Amanieu:master Apr 30, 2023
37 checks passed
@daxpedda
Copy link
Contributor

I'm surprised worker names are used as an identifier here, worker names are not unique in any way and often left empty. So this deadlock detector will just not work most of the time.

Would a PR be acceptable that simply uses an atomic thread counter?

@daxpedda
Copy link
Contributor

After looking at this PR again, I noticed that it used cfg(target = "..."), which is currently an unstable feature (RFC), which wasn't activated.

Additionally, it wasn't caught because the build_other_platforms CI, the only one testing Wasm, didn't test this feature.

daxpedda added a commit to daxpedda/parking_lot that referenced this pull request May 15, 2023
@daxpedda
Revert "Merge pull request Amanieu#379 from samdenty/master"
065e947 
This reverts commit de2a402, reversing
changes made to aa6c4cc.
daxpedda added a commit to daxpedda/parking_lot that referenced this pull request May 15, 2023
@daxpedda
Revert Amanieu#379
09ca773
@daxpedda daxpedda mentioned this pull request May 15, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Amanieu Amanieu Amanieu left review comments

@bjorn3 bjorn3 bjorn3 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@samdenty @Amanieu @daxpedda @bjorn3