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.

Sign up for GitHub

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

Jump to bottom

Prevent deadlock by not using futures::executor::block_on #1121

Merged
merged 3 commits into from
Nov 19, 2024
Merged

Prevent deadlock by not using futures::executor::block_on #1121

merged 3 commits into from
Nov 19, 2024

Commits on Nov 18, 2024

  1. Avoid futures::executor::block_on in coordinator <-> worker IO 

    As Alice Rhyl recommended (slightly reworded)

> Either you can use `tokio::runtime::Handle::block_on` [instead of
> `futures::executor::block_on`] or your program has a subtle
> deadlock.
    @shepmaster
    shepmaster committed Nov 18, 2024
    Configuration menu
    Copy the full SHA
    08a7449 View commit details
    Browse the repository at this point in the history

  2. Avoid using futures' block_on in the request database guard 

    The request guard works by sending on a channel when it is
dropped. Since we can't do async code in `Drop::drop`, I was using
`futures::executor::block_on` under the (false!) impression that
this *specific* usecase would not be a problem as we are always
pulling from that channel. However, sending on a channel can result in
`Pending` when the Tokio worker's cooperative budget is exhausted,
even if the queue is completely empty!

A simple example:

```rust
// Drain the budget
while task::consume_budget().now_or_never().is_some() {}

// Try to use more budget than we have and block forever
futures::executor::block_on(tx.send(42u8)).unwrap();
```

This `Pending` will never resolve because we are no longer running the
Tokio runtime so it will never see an `.await` and replenish the
budget. With enough bad luck, all the worker threads get stuck like
this and the entire system becomes unavailable.

There are a number of possible workarounds, but the least invasive is
to punt the final logging off to a Tokio task and let it happen
out-of-band. Technically this will make the timing a little bit less
accurate, but I was never worried about exact nanoseconds anyway.
    @shepmaster
    shepmaster committed Nov 18, 2024
    Configuration menu
    Copy the full SHA
    24decf1 View commit details
    Browse the repository at this point in the history

  3. Take down the server if the request logging fails

    @shepmaster
    shepmaster committed Nov 18, 2024
    Configuration menu
    Copy the full SHA
    13631b4 View commit details
    Browse the repository at this point in the history