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

JoinHandle::join doesn't document interaction with atomics #45467

Closed
hniksic opened this issue Oct 23, 2017 · 4 comments
Closed

JoinHandle::join doesn't document interaction with atomics #45467

hniksic opened this issue Oct 23, 2017 · 4 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority

Comments

@hniksic
Copy link
Contributor

hniksic commented Oct 23, 2017

The current documentation of JoinHandle::join doesn't specify the effect of joining the thread on the atomics that the thread modified. I couldn't find any other place in the documentation where this would be described either.

Without additional guarantees, the call to AtomicUsize::into_inner in the following snippet could be understood to constitute a data race:

let i_ref = Arc::new(AtomicUsize::new(0));
let thr = std::thread::spawn({
    let i_ref = i_ref.clone();
    move || i_ref.store(1, Ordering:: Relaxed)
});
thr.join().unwrap();
let i = Arc::try_unwrap(i_ref).unwrap().into_inner();
assert_eq!(i, 1);

In comparison, cppreference explicitly states that "The completion of the thread identified by *this synchronizes with the corresponding successful return from join()."

The nomicon does claim that Rust just inherits C11's memory model for atomics, which could be used to infer that JoinHandle::join has the same effect as C++'s std::thread::join. It would still be a good idea to document the synchronizes-with relation explicitly to avoid confusion such as in this stackoverflow question.

@kennytm kennytm added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Oct 23, 2017
@TimNN TimNN added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Oct 24, 2017
@steveklabnik
Copy link
Member

@rust-lang/libs @rust-lang/lang , can we get a clarification on intended semantics here?

@steveklabnik steveklabnik added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 31, 2017
@alexcrichton
Copy link
Member

Discussed during libs triage today, the conclusion was to just document what C++ does as we're built on the same primitives

@steveklabnik steveklabnik added P-medium Medium priority and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 21, 2017
@RalfJung
Copy link
Member

Should be fixed by #53389... @hniksic could you have a look?

@hniksic
Copy link
Contributor Author

hniksic commented Aug 17, 2018

@RalfJung Looks great, thanks!

@hniksic hniksic closed this as completed Aug 17, 2018
kennytm added a commit to kennytm/rust that referenced this issue Aug 28, 2018
document effect of join on memory ordering

Fixes rust-lang#45467
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Aug 28, 2018
document effect of join on memory ordering

Fixes rust-lang#45467
pietroalbini added a commit to pietroalbini/rust that referenced this issue Aug 29, 2018
document effect of join on memory ordering

Fixes rust-lang#45467
pietroalbini added a commit to pietroalbini/rust that referenced this issue Aug 29, 2018
document effect of join on memory ordering

Fixes rust-lang#45467
pietroalbini added a commit to pietroalbini/rust that referenced this issue Aug 30, 2018
document effect of join on memory ordering

Fixes rust-lang#45467
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

6 participants