Skip to content

Commit

Permalink
fix(hub): avoid deadlocks when emitting events (#633)
Browse files Browse the repository at this point in the history
In a very specific combination of events, any event emission can
deadlock if it tries to add stuff to the Hub, like breadcrumbs. This can
be easily reproduced with the following setup:

- `sentry-log` (or `sentry-tracing` if `tracing-log` is set up)
- event filtering configured to generate breadcrumbs for debug logs
- any cause for events to not be sent to the server

Should the event fail to be sent for too long, the channel to the sender
thread fills up and events start to be dropped. This will generate a
debug log line, logging that an event has been dropped and why. With
`sentry-log` (or `tracing-log` + `sentry-tracing`), this would generate
a breadcrumb.

However, the whole call to `Transport::send_envelope` is done in the
context of `HubImpl::with`, that holds a read lock on the `HubImpl`'s
stack. When generating the breadcrumb for the debug line, we end up
calling `Hub::add_breadcrumb`, which calls `HubImpl::with_mut` to get a
mutable reference to the top of the stack. However, since we already
have a read lock on the stack, we deadlock on the write lock.

The fix is to move the call to `Transport::send_envelope` outside of the
lock zone, and we use `HubImpl::with` only to clone the top
`StackLayer`. Since this structure is only `Arc`s, the only performance
hit is two `Arc` clones and not the whole stack cloning.
  • Loading branch information
Tuetuopay committed Jan 3, 2024
1 parent 02b708a commit acba14f
Showing 1 changed file with 5 additions and 10 deletions.
15 changes: 5 additions & 10 deletions sentry-core/src/hub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,11 @@ impl Hub {
/// for more documentation.
pub fn capture_event(&self, event: Event<'static>) -> Uuid {
with_client_impl! {{
self.inner.with(|stack| {
let top = stack.top();
if let Some(ref client) = top.client {
let event_id = client.capture_event(event, Some(&top.scope));
*self.last_event_id.write().unwrap() = Some(event_id);
event_id
} else {
Default::default()
}
})
let top = self.inner.with(|stack| stack.top().clone());
let Some(ref client) = top.client else { return Default::default() };
let event_id = client.capture_event(event, Some(&top.scope));
*self.last_event_id.write().unwrap() = Some(event_id);
event_id
}}
}

Expand Down

0 comments on commit acba14f

Please sign in to comment.