getsentry / sentry-rust

Official Sentry SDK for Rust
https://sentry.io/
Apache License 2.0
620 stars 153 forks source link

fix(hub): avoid deadlocks when emitting events #633

Closed Tuetuopay closed 10 months ago

Tuetuopay commented 10 months ago

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:

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 Arcs, the only performance hit is two Arc clones and not the whole stack cloning.

We hit this in prod because (for reasons) we had to backport the legacy pre-envelope transport, and this one uses the warning level when the channel is full. The default log filter is safe from this deadlock because debug events are dropped, but anyone enabling at least breadcrumbs for debug events are exposed to it.

Thanks!

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (02b708a) 73.07% compared to head (c6a4fd8) 73.07%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #633 +/- ## ========================================== - Coverage 73.07% 73.07% -0.01% ========================================== Files 62 62 Lines 7494 7490 -4 ========================================== - Hits 5476 5473 -3 + Misses 2018 2017 -1 ```