From acba14fb89638749a2b662223946498f92c4008c Mon Sep 17 00:00:00 2001 From: Tuetuopay Date: Wed, 3 Jan 2024 16:38:41 +0100 Subject: [PATCH] fix(hub): avoid deadlocks when emitting events (#633) 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. --- sentry-core/src/hub.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/sentry-core/src/hub.rs b/sentry-core/src/hub.rs index ea61386f..cc0a9c3d 100644 --- a/sentry-core/src/hub.rs +++ b/sentry-core/src/hub.rs @@ -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 }} }