fix: Explicitly drop SessionFlusher (#326)

It seems that doing a thread::join from within the Drop impl of a thread
local THREAD_HUB will deadlock for some reason.
Explicitly joining the background flusher as part of the ClientGuard
Drop does work, and is also more correct, as otherwise the PROCESS_HUB
might keep the worker alive forever.

Author: Swatinem

Diff for: sentry-core/src/client.rs

@@ -43,7 +43,7 @@ pub(crate) type TransportArc = Arc<RwLock<Option<Arc<dyn Transport>>>>;
 pub struct Client {
     options: ClientOptions,
     transport: TransportArc,
-    session_flusher: SessionFlusher,
+    session_flusher: RwLock<Option<SessionFlusher>>,
     integrations: Vec<(TypeId, Arc<dyn Integration>)>,
     sdk_info: ClientSdkInfo,
 }
@@ -60,7 +60,10 @@ impl fmt::Debug for Client {
 impl Clone for Client {
     fn clone(&self) -> Client {
         let transport = Arc::new(RwLock::new(self.transport.read().unwrap().clone()));
-        let session_flusher = SessionFlusher::new(transport.clone(), self.options.session_mode);
+        let session_flusher = RwLock::new(Some(SessionFlusher::new(
+            transport.clone(),
+            self.options.session_mode,
+        )));
         Client {
             options: self.options.clone(),
             transport,
@@ -128,7 +131,10 @@ impl Client {
             sdk_info.integrations.push(integration.name().to_string());
         }
 
-        let session_flusher = SessionFlusher::new(transport.clone(), options.session_mode);
+        let session_flusher = RwLock::new(Some(SessionFlusher::new(
+            transport.clone(),
+            options.session_mode,
+        )));
         Client {
             options,
             transport,
@@ -287,12 +293,16 @@ impl Client {
     }
 
     pub(crate) fn enqueue_session(&self, session_update: SessionUpdate<'static>) {
-        self.session_flusher.enqueue(session_update)
+        if let Some(ref flusher) = *self.session_flusher.read().unwrap() {
+            flusher.enqueue(session_update);
+        }
     }
 
     /// Drains all pending events without shutting down.
     pub fn flush(&self, timeout: Option<Duration>) -> bool {
-        self.session_flusher.flush();
+        if let Some(ref flusher) = *self.session_flusher.read().unwrap() {
+            flusher.flush();
+        }
         if let Some(ref transport) = *self.transport.read().unwrap() {
             transport.flush(timeout.unwrap_or(self.options.shutdown_timeout))
         } else {
@@ -308,7 +318,7 @@ impl Client {
     /// If no timeout is provided the client will wait for as long a
     /// `shutdown_timeout` in the client options.
     pub fn close(&self, timeout: Option<Duration>) -> bool {
-        self.session_flusher.flush();
+        drop(self.session_flusher.write().unwrap().take());
         let transport_opt = self.transport.write().unwrap().take();
         if let Some(transport) = transport_opt {
             sentry_debug!("client close; request transport to shut down");

Diff for: sentry-core/src/hub.rs

@@ -156,7 +156,7 @@ impl Hub {
             f(&PROCESS_HUB.0)
         } else {
             // note on safety: this is safe because even though we change the Arc
-            // by temorary binding we guarantee that the original Arc stays alive.
+            // by temporary binding we guarantee that the original Arc stays alive.
             // For more information see: run
             THREAD_HUB.with(|stack| unsafe {
                 let ptr = stack.get();

Diff for: sentry/Cargo.toml

@@ -51,7 +51,7 @@ log_ = { package = "log", version = "0.4.8", optional = true, features = ["std"]
 reqwest_ = { package = "reqwest", version = "0.11", optional = true, features = ["blocking", "json"], default-features = false }
 curl_ = { package = "curl", version = "0.4.25", optional = true }
 surf_ = { package = "surf", version = "2.0.0", optional = true }
-httpdate = { version = "0.3.2", optional = true }
+httpdate = { version = "1.0.0", optional = true }
 serde_json = { version = "1.0.48", optional = true }
 tokio = { version = "1.0", features = ["rt"] }
 

Diff for: sentry/tests/test_client.rs

@@ -56,3 +56,18 @@ fn test_unwind_safe() {
 
     assert_eq!(events.len(), 1);
 }
+
+#[test]
+fn test_concurrent_init() {
+    let _guard = sentry::init(sentry::ClientOptions {
+        ..Default::default()
+    });
+
+    std::thread::spawn(|| {
+        let _guard = sentry::init(sentry::ClientOptions {
+            ..Default::default()
+        });
+    })
+    .join()
+    .unwrap();
+}