Skip to content

Commit

Permalink
Avoid using futures' block_on in the request database guard
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
shepmaster committed Nov 18, 2024
1 parent 08a7449 commit 24decf1
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions ui/src/request_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ impl Handle {
let g = self
.attempt_start_request(category, payload)
.await
.map(|id| EndGuardInner(id, How::Abandoned, self));
.map(|id| EndGuardInner(id, How::Abandoned, Some(self)));
EndGuard(g)
}
}
Expand All @@ -238,12 +238,16 @@ impl EndGuard {
}
}

struct EndGuardInner(Id, How, Handle);
struct EndGuardInner(Id, How, Option<Handle>);

impl Drop for EndGuardInner {
fn drop(&mut self) {
let Self(id, how, ref handle) = *self;
futures::executor::block_on(handle.attempt_end_request(id, how))
let Self(id, how, ref mut handle) = *self;
if let Ok(h) = tokio::runtime::Handle::try_current() {
if let Some(handle) = handle.take() {
h.spawn(async move { handle.attempt_end_request(id, how).await });
}
}
}
}

Expand Down

0 comments on commit 24decf1

Please sign in to comment.