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 no-threads mode, for use in single-threaded environments #82

Closed
wants to merge 1 commit into from

Conversation

matklad
Copy link
Owner

@matklad matklad commented Jan 1, 2020

No description provided.

Copy link

@josephlr josephlr left a comment

Choose a reason for hiding this comment

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

This is awesome, we would like to use this (instead of unsafe code) in https://github.com/cloud-hypervisor/rust-hypervisor-firmware.

I reviewed the changes and they seem correct and compatible with our use-cases. However, I think the name of the feature should be: no-std-panic-on-contention.

This better reflects that this implementation doesn't prohibit threading. It just panic's if you try to initialize the cell while initializing it from another thread. Alternatively, this doesn't even have to be a Cargo feature, it could just be the documented behavior of this crate if std isn't enabled.

There are many use cases (setting up page tables, IDTs, initializing IO, etc...) where you need to do some one-time setup in a single-threaded context. After that setup succeeds, you might access the object immutably from other threads.

Finally, something like cargo build --no-default-features --features "no-std-panic-on-contention" should be added to .travis.yml so that this feature doesn't break.

@@ -228,7 +228,7 @@ might be easier to debug than a deadlock.
#[path = "imp_pl.rs"]
mod imp;

#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "assume-no-threads-or-interrupts"))]

Choose a reason for hiding this comment

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

It would be nice if we could just declare the imp module inside the sync module, so we don't have to worry about these cfgs getting out of sync.

However, rust-lang/rust#35016 is still not fixed, so the only way to do this is moving the imp_*.rs files (which seems like a hassle).

@matklad
Copy link
Owner Author

matklad commented Nov 11, 2020

#118 is a better soln than this, available in 1.5.1

@matklad matklad closed this Nov 11, 2020
@matklad matklad deleted the no-threads-or-interrupts branch February 27, 2021 16:05
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.

2 participants