Skip to content

Document use of future::timeout in combination with channels #411

Open
@yoshuawuyts

Description

@yoshuawuyts
Contributor

As shared here, channels could potentially deadlock if the capacity isn't large enough:

let (s, r) = channel(1);

s.send(1).await;
s.send(2).await; // this will hang indefinitely

Instead we should probably mention that by using future::timeout this can be prevented from hanging indefinitely:

let (s, r) = channel(1);

future::timeout(s.send(1), Duration::from_secs(1).await);
future::timeout(s.send(2), Duration::from_secs(1).await);

Perhaps we could even mention in the docs that folks should probably default to something spacious such as 256 or 1024 messages in case of doubt. In general we probably shouldn't assume people have intuition about channels, and nudging them towards some safer default choices (because unlike with unbounded channels they have to choose) wouldn't be a bad idea probably (:

Activity

Matthias247

Matthias247 commented on Oct 31, 2019

@Matthias247

Perhaps we could even mention in the docs that folks should probably default to something spacious such as 256 or 1024 messages in case of doubt. In general we probably shouldn't assume people have intuition about channels, and nudging them towards some safer default choices (because unlike with unbounded channels they have to choose) wouldn't be a bad idea probably (:

Please not - that is the wrong advice. The outcome of this is that the system will then just deadlock under heavy load instead of early on. This will be even harder to detect, diagnose and fix. And it will exactly happen when you need it at least: In production under stress. Channels should be be used in a fashion where they can't deadlock independent of the size. As the Uber Go Styleguide mentions: Channels should be sized by default of size 0 or 1. All other sizes can be used to improve throughput by helping to buffer across some switching latencies, but they shouldn't be used to fix architecture. They will also in doubt increase tail latencies.

The issues should be fixed with having the right architecture, which e.g. means producer and consumer shouldn't run on the same thread/task, and there should not be cyclic dependencies. If there are cyclic dependencies (e.g. a return channel) then that channel should be used in a very limited and deterministic fashion. E.g. it might be used exactly once and have a capacity of 1 (oneshot channels are good for this). For other use-cases where producer and consumer are on different threads rendezvous channels (size 0) guarantee the highest chance for correctness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentation

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @yoshuawuyts@Matthias247

        Issue actions

          Document use of future::timeout in combination with channels · Issue #411 · async-rs/async-std