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

[docs] Mutex Example section: FIFO ordering explanation wrong #3614

Closed
setpill opened this issue Mar 12, 2021 · 2 comments · Fixed by #3615
Closed

[docs] Mutex Example section: FIFO ordering explanation wrong #3614

setpill opened this issue Mar 12, 2021 · 2 comments · Fixed by #3615
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue. M-sync Module: tokio/sync T-docs Topic: documentation

Comments

@setpill
Copy link
Contributor

setpill commented Mar 12, 2021

/// ```rust,no_run
/// use tokio::sync::Mutex;
/// use std::sync::Arc;
///
/// #[tokio::main]
/// async fn main() {
/// let count = Arc::new(Mutex::new(0));
///
/// for _ in 0..5 {
/// let my_count = Arc::clone(&count);
/// tokio::spawn(async move {
/// for _ in 0..10 {
/// let mut lock = my_count.lock().await;
/// *lock += 1;
/// println!("{}", lock);
/// }
/// });
/// }
///
/// loop {
/// if *count.lock().await >= 50 {
/// break;
/// }
/// }
/// println!("Count hit 50.");
/// }
/// ```

/// Tokio's Mutex works in a simple FIFO (first in, first out) style where all
/// calls to [`lock`] complete in the order they were performed. In that way the
/// Mutex is "fair" and predictable in how it distributes the locks to inner
/// data. This is why the output of the program above is an in-order count to
/// 50. Locks are released and reacquired after every iteration, so basically,

This does not line up. The in-order count seems to be a consequence of the println happening before the lock is released. If Tokio's Mutex worked in a FILO order it would not count backwards.

@setpill setpill added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Mar 12, 2021
@setpill
Copy link
Contributor Author

setpill commented Mar 12, 2021

Comment from @Darksonn on Discord:

If you modify it like this, you would see the effect of the fairness though:

for i in 0..5 {
    let my_count = Arc::clone(&count);
    tokio::spawn(async move {
        for _ in 0..10 {
            let mut lock = my_count.lock().await;
            *lock += 1;
            println!("{} {}", i, lock);
        }
    });
}

@Darksonn Darksonn added E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue. M-sync Module: tokio/sync T-docs Topic: documentation labels Mar 12, 2021
@setpill
Copy link
Contributor Author

setpill commented Mar 12, 2021

Implemented the above snippet (added a j to the inner loop as well). Its output:

$ cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/tokiotest`
0 0 1
1 0 2
0 1 3
2 0 4
4 0 5
3 0 6
1 1 7
0 2 8
2 1 9
4 1 10
3 1 11
1 2 12
0 3 13
2 2 14
4 2 15
3 2 16
1 3 17
0 4 18
2 3 19
4 3 20
3 3 21
1 4 22
0 5 23
2 4 24
4 4 25
3 4 26
1 5 27
0 6 28
2 5 29
4 5 30
3 5 31
1 6 32
0 7 33
2 6 34
4 6 35
3 6 36
1 7 37
0 8 38
2 7 39
4 7 40
3 7 41
1 8 42
0 9 43
2 8 44
4 8 45
3 8 46
1 9 47
2 9 48
4 9 49
3 9 50
Count hit 50.

The ordering changes between runs, and it definitely is not guaranteed to happen in source code order, that's for sure 😄 I think it would be better to substitute "predictable" with "deterministic".

Edit: upon closer inspection, it seems that the unpredictability in the order mostly comes from the start where some threads already start running before the others are set up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue. M-sync Module: tokio/sync T-docs Topic: documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants