alternative timer compatibility hazard

[vilgotf]

Version
1.49.0 and above (since the alternative timer was added)

Platform
N/A

Description
The alt timer does not retain the previous waker in Sleep::reset. When sharing Sleep and DelayQueue between tasks (e.g. Arc<Mutex<Pin<Box<Sleep>>>>), a task may lose its waker if another task calls reset. Waker retention was added in #2217 and #2290 (back in 0.2).

Rather than fixing the alternative timer, I would rather remove these spurious wakes and label cross-task sharing unsupported. This would be a minor compatibility break, as I doubt anyone actually uses this functionality (and whether we even knew about it). Alternatively, this should at least be internally documented, and perhaps publicly as well.

With this removed from Sleep, we would no longer need to store Option<Waker> in DelayQueue. That waker is only necessary to wake the task currently calling poll_expired after insert/remove operations (allowing it to reschedule itself).

Discovered in #8137, when I tried to normalize both implementations. This was not caught by our tests, as the alternative timer requires the multi-threaded runtime but our tests run on the current-threaded runtime.

[vilgotf]

It seems DelayQueue::remove wakes was added just 2 years ago (#6751) and that would therefore likely cause breakage. But removal could be scheduled with a breaking release.

Given the above, however, I doubt that I can convince everyone of this breaking change.

[ADD-SP]

Currently, Tokio is one of the few crates I use that I feel comfortable to run cargo update -p. I’ve encountered many dependencies where even a patch version update introduced behavioral changes, which causes non-enjoyable experience to me.

Thus, I still prefer keeping this behavior unless we have a strong reason to break it. For example, significant complexity for implementation, security concern.

But, to be honest, Arc<Mutex<Pin<Box<Sleep>>>> looks terrible, and I think a reasonable async application should not rely on this behavior. So I don't think my argument is very strong and I'm open for debating.

@Darksonn Could you share your thought?

[vilgotf]

I found 4 repos using Mutex<DelayQueue>: https://github.com/search?q=Mutex%3CDelayQueue%3E&type=code (3 of which are abandoned and the other being an interview challenge)
I found 2 repos using Mutex<Pin<Box<Sleep>>: https://github.com/search?q=Mutex%3CPin%3CBox%3CSleep%3E%3E%3E&type=code (one is abandoned, but other would break @toidiu)

---

The original motivation behind #2290 was for Mutex<DelayQueue>; supporting Mutex<Pin<Box<Sleep>> was likely accidental. It might therefore be possible to retain Mutex<DelayQueue> support while dropping Mutex<Pin<Box<Sleep>>> support. Note that removing Mutex<Pin<Box<Sleep>>> breaks Mutex<DelayQueue>, so both must be updated in tandem.

---

Thus, I still prefer keeping this behavior unless we have a strong reason to break it. For example, significant complexity for implementation, security concern.

I don't know how much additional complexity is required since I've not written a patch yet, but another consequence of retaining support would be spurious wakes and their accompanying performance hit.