From 42611c9d904e66d8e1a26ae058a02dcfd227f7ef Mon Sep 17 00:00:00 2001 From: boxdot Date: Thu, 5 Dec 2024 10:16:40 +0100 Subject: [PATCH 1/2] fix: deadlock when replacing stream sink in SendToDartLogger When setting a new stream sink in `SendToDartLogger`, we lock the global rw lock for writing. However, if the lock already contained a sink, it was droppen at the same line when replacing it. A stream sink logs a message on drop, and therefore `SendToDartLogger` tries to lock the global rw lock again, this time for reading. The lock is not reentrant, so it deadlocks. This makes sure we drop the previous sink *after* unlocking the rw lock. Fixes #225 --- applogic/src/api/mobile_logging.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/applogic/src/api/mobile_logging.rs b/applogic/src/api/mobile_logging.rs index 9f7cc39f..73d7c999 100644 --- a/applogic/src/api/mobile_logging.rs +++ b/applogic/src/api/mobile_logging.rs @@ -79,14 +79,13 @@ pub struct SendToDartLogger { impl SendToDartLogger { pub fn set_stream_sink(stream_sink: StreamSink) { - let mut guard = SEND_TO_DART_LOGGER_STREAM_SINK.write().unwrap(); - let overriding = guard.is_some(); - - *guard = Some(stream_sink); - - drop(guard); - - if overriding { + let prev_stream_sink = { + let mut guard = SEND_TO_DART_LOGGER_STREAM_SINK.write().expect("poisoned"); + // Note: previous stream sink MUST NOT be dropped before the `guard` is released. + // On drop, it will log and because the `RwLock` is not reentrant, this will deadlock. + guard.replace(stream_sink) + }; + if prev_stream_sink.is_some() { warn!( "SendToDartLogger::set_stream_sink but already exist a sink, thus overriding. \ (This may or may not be a problem. It will happen normally if hot-reload Flutter app.)" From a31311b65d3a841985eb0a86700b7fe618706959 Mon Sep 17 00:00:00 2001 From: boxdot Date: Thu, 5 Dec 2024 13:00:23 +0100 Subject: [PATCH 2/2] clarify comment --- applogic/src/api/mobile_logging.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/applogic/src/api/mobile_logging.rs b/applogic/src/api/mobile_logging.rs index 73d7c999..ec0aa57a 100644 --- a/applogic/src/api/mobile_logging.rs +++ b/applogic/src/api/mobile_logging.rs @@ -81,8 +81,9 @@ impl SendToDartLogger { pub fn set_stream_sink(stream_sink: StreamSink) { let prev_stream_sink = { let mut guard = SEND_TO_DART_LOGGER_STREAM_SINK.write().expect("poisoned"); - // Note: previous stream sink MUST NOT be dropped before the `guard` is released. - // On drop, it will log and because the `RwLock` is not reentrant, this will deadlock. + // Note: The previous stream sink MUST NOT be dropped before the `guard` is released. + // On drop, it will log a message, and therefore lock the sink for reading. Because the + // `RwLock` is not reentrant, this will deadlock. guard.replace(stream_sink) }; if prev_stream_sink.is_some() {