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

Once is racy or ill-defined #1

Closed
briansmith opened this issue Jan 2, 2020 · 1 comment
Closed

Once is racy or ill-defined #1

briansmith opened this issue Jan 2, 2020 · 1 comment

Comments

@briansmith
Copy link

The documentation for Once says "This type is very similar to the std::sync::Once type in the standard library, but features a richer API." From this statement, I gather that conquer_once::Once is intended to fully subsume the functionality of std::sync::Once. However, this isn't the case, AFAICT, because std::sync::Once::call_once() guarantees "that any memory writes performed by the executed closure can be reliably observed by other threads at this point (there is a happens-before relation between the closure and code executing after the return)." So, in particular, if the closure you pass to std::sync::Once::call_once writes to (global) memory before it returns, those writes to global memory will be observed by other threads.

conquer-once::sync::OnceCell does NOT provide that guarantee, but Once is defined as an alias of OnceCell<()>. So either OnceCell needs to start guaranteeing the same thing as std::sync::Once or conquer_once::Once needs to be reimplemented to not use OnceCell as its implementation.

Further, the documentation for OnceCell needs to be clarified regarding whether or not this guarantee is present.

See briansmith/ring#924 and briansmith/ring#931 for a real-world example that would have been negatively affected by this.

@oliver-giersch
Copy link
Owner

Sorry I completely missed this issue and only stumbled across this now by accident. I double checked the implementations both in for std::sync::Once and this crate and they provide the same guarantees (i.e. a Release fence is issued before the closure completes) so this is merely an issue of documentation.

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

No branches or pull requests

2 participants