sync: implement 'Clone' for watch::Sender

[bouk]

Here I'm implement Clone for watch::Sender, which makes it mpsc

TODO

• Update docs to make it clear it's mpmc

Motivation

See #4809

Solution

I've implemented one of the suggestions in the issue, which is to keep track of the number of Senders separate from the version.

Open questions: are there other methods that need to be added? E.g. 'upgrading' a receiver to a sender or getting the number of current Senders

[bouk]

This method seems to not match maybe_changed because the order of checking whether it's closed and the version has changed is different. The behavior in other places is that it will give one last true when all the Senders are gone before returning an error, but here it gives an error even when there's an unobserved change.

Maybe I should change it to match behavior?

[wathenjiang]

I believe the inconsistency does exists, but

• the current version do exactly as the comment say, Returns an error if the channel has been closed.
• maybe_changed is not a pub method.

The impact of inconsistency may be not particularly large.

But the maybe_changed method does affect the behavior of pub method changed. The comments of changed should be modified to

This method returns an error if there is no new value in the channel has not yet been marked seen and all [`Sender`]s are dropped.

from

This method returns an error if and only if the [`Sender`] is dropped.

[Darksonn]

Since you've split the closed bit out from the atomic, this introduces extra possibilities for race conditions causing incorrect behavior. I would need some time to think about whether this is still correct.

[hawkw]

It's interesting that the AtomicWaker loom tests are failing on this PR, when this branch hasn't actually changed AtomicWaker: https://github.com/tokio-rs/tokio/actions/runs/5923638182/job/16059659886?pr=5936#step:5:58

[Darksonn]

This fails the watch smoke test:

thread 'sync::tests::loom_watch::smoke' panicked at 'called `Result::unwrap()` on an `Err` value: RecvError(())', tokio/src/sync/tests/loom_watch.rs:20:33
stack backtrace:
   0: rust_begin_unwind
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/panicking.rs:67:14
   2: core::result::unwrap_failed
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/result.rs:1651:5
   3: core::ops::function::FnOnce::call_once{{vtable.shim}}
   4: generator::stack::StackBox<F>::call_once
   5: std::panicking::try
   6: generator::gen_impl::gen_init
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
test sync::tests::loom_watch::smoke ... FAILED

[Darksonn]

@hawkw Those are tests that are supposed to panic, so they print a panic but don't fail.

[Darksonn]

This implementation is currently incorrect. I'll close this PR since there has been no activity for a while.

In either case, thanks for taking the time to submit a PR. If you want to continue working on it in the future, then please feel free to reopen or post a new PR.