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

Add spinlock implementation without std::thread::sleep #138

Merged
merged 4 commits into from
Aug 15, 2024

Conversation

white-axe
Copy link
Contributor

@white-axe white-axe commented Dec 21, 2023

This pull request adds a spin-plain feature that uses a spinlock-based locking system like the spin feature does, but without using std::thread::sleep. Closes #137.
Edit: This pull request changes the spin feature to only use std::thread::sleep on Unix and Windows. Closes #137.

Some targets like wasm32-unknown-unknown don't support blocking, so you won't be able to use flume on those platforms without this new feature. Even if you enable -Ctarget-feature=+atomics,+bulk-memory,+mutable-globals on the wasm32-unknown-unknown target, flume still doesn't work on the main thread of a web browser, it only works in web workers. See #137 for more details about this, but essentially the problem is that the wait_lock function in flume has two different implementations and both of them are blocking, including the current spinlock-based one.

@Speak2Erase
Copy link

@zesterer ?

Copy link
Owner

@zesterer zesterer left a comment

Choose a reason for hiding this comment

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

Thanks for the reminder, this PR flew under the radar.

Cargo.toml Outdated Show resolved Hide resolved
@white-axe white-axe requested a review from zesterer July 28, 2024 01:38
@zesterer
Copy link
Owner

Great, thanks for the PR!

@Speak2Erase
Copy link

??

@sug0
Copy link

sug0 commented Aug 13, 2024

maybe a ci wasm build check would be nice? to guarantee that doesn't break again

@Speak2Erase
Copy link

@zesterer ??

@zesterer zesterer merged commit 3c47bcc into zesterer:master Aug 15, 2024
5 checks passed
@zesterer
Copy link
Owner

Sorry about that. I replied, ran CI, then didn't get back to this. Anyhow, much appreciated!

@white-axe white-axe deleted the plain branch August 15, 2024 14:52
@Speak2Erase
Copy link

Tysm!! Would it be possible to get this published on crates.io as well?

@Speak2Erase
Copy link

@zesterer I'm sorry for constantly pinging you- but it seems to be the only way to get your attention?

@zesterer
Copy link
Owner

I should have some free time tomorrow to do this, thanks for the reminder.

@Speak2Erase
Copy link

@zesterer ?

@zesterer
Copy link
Owner

zesterer commented Sep 2, 2024

Not forgotten about this, trying to find the time to write up what turns out to be quite an extensive changelog!

@sug0
Copy link

sug0 commented Sep 2, 2024

take your time, we appreciate your efforts :-)

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.

Senders and receivers are both unusable in browser main thread in WASM
4 participants