diff --git a/.changes/linux-taskrunner-use-after-free b/.changes/linux-taskrunner-use-after-free new file mode 100644 index 000000000..f3066b634 --- /dev/null +++ b/.changes/linux-taskrunner-use-after-free @@ -0,0 +1 @@ +patch type="fixed" "Fix use-after-free crash in TaskRunnerLinux::EnqueueTask when the runner is destroyed before the main loop dispatches a queued task" diff --git a/linux/livekit_plugin.cpp b/linux/livekit_plugin.cpp index 4c29d5e92..158843c70 100644 --- a/linux/livekit_plugin.cpp +++ b/linux/livekit_plugin.cpp @@ -73,7 +73,7 @@ class VisualizerSink : public libwebrtc::AudioTrackSink { &flutter::StandardMethodCodec::GetInstance())), media_track_(media_track), is_centered_(is_centered), bar_count_(bar_count) { - task_runner_ = std::make_unique(); + task_runner_ = std::make_shared(); auto handler = std::make_unique< flutter::StreamHandlerFunctions>( [&](const flutter::EncodableValue *arguments, @@ -151,7 +151,7 @@ class VisualizerSink : public libwebrtc::AudioTrackSink { private: std::unique_ptr audio_visualizer_; - std::unique_ptr task_runner_; + std::shared_ptr task_runner_; std::unique_ptr> channel_; std::shared_ptr> sink_; std::list event_queue_; @@ -172,7 +172,7 @@ class AudioRendererSink : public libwebrtc::AudioTrackSink { messenger, event_channel_name, &flutter::StandardMethodCodec::GetInstance())), media_track_(media_track), format_(format) { - task_runner_ = std::make_unique(); + task_runner_ = std::make_shared(); auto handler = std::make_unique< flutter::StreamHandlerFunctions>( [&](const flutter::EncodableValue *arguments, @@ -250,7 +250,7 @@ class AudioRendererSink : public libwebrtc::AudioTrackSink { } } - std::unique_ptr task_runner_; + std::shared_ptr task_runner_; std::unique_ptr> channel_; std::shared_ptr> sink_; bool on_listen_called_ = false; diff --git a/linux/task_runner_linux.cc b/linux/task_runner_linux.cc index 4beb298b4..c0054fab8 100644 --- a/linux/task_runner_linux.cc +++ b/linux/task_runner_linux.cc @@ -12,19 +12,44 @@ void TaskRunnerLinux::EnqueueTask(TaskClosure task) { GMainContext* context = g_main_context_default(); if (context) { - g_main_context_invoke( - context, - [](gpointer user_data) -> gboolean { - TaskRunnerLinux* runner = static_cast(user_data); - std::lock_guard lock(runner->tasks_mutex_); - while (!runner->tasks_.empty()) { - TaskClosure task = std::move(runner->tasks_.front()); - runner->tasks_.pop(); - task(); + // A weak_ptr (not `this`) is passed through so that if the runner is + // destroyed before the main loop dispatches this callback, for example + // when the owning sink is torn down while an audio frame's task is still + // queued, the callback safely no-ops instead of locking a mutex inside + // freed memory. g_main_context_invoke_full's notify always runs exactly + // once, whether the callback fired inline or via the idle source, so the + // heap-allocated weak_ptr is never leaked. + // + // weak_from_this() itself is only safe here because the owning sink is + // destroyed after the audio track's RemoveSink() returns, and RemoveSink() + // blocks until any in-flight OnData() (and therefore this call) has + // finished. Callers must keep that ordering. + auto* weak_self = new std::weak_ptr(weak_from_this()); + g_main_context_invoke_full( + context, G_PRIORITY_DEFAULT, + [](gpointer user_data) -> gboolean { + auto* weak_self = static_cast*>(user_data); + if (auto runner = weak_self->lock()) { + // Take the whole batch under the lock, then run it unlocked so a + // task that re-enters EnqueueTask on this runner cannot deadlock + // on the non-recursive mutex. + std::queue pending; + { + std::lock_guard lock(runner->tasks_mutex_); + std::swap(pending, runner->tasks_); + } + while (!pending.empty()) { + TaskClosure task = std::move(pending.front()); + pending.pop(); + task(); + } } return G_SOURCE_REMOVE; }, - this); + weak_self, + [](gpointer user_data) { + delete static_cast*>(user_data); + }); } } diff --git a/linux/task_runner_linux.h b/linux/task_runner_linux.h index 9cd400a1d..9d1ab3fa0 100644 --- a/linux/task_runner_linux.h +++ b/linux/task_runner_linux.h @@ -4,13 +4,19 @@ #include #include #include - #include - - using TaskClosure = std::function; +#include + +using TaskClosure = std::function; namespace livekit_client_plugin { -class TaskRunnerLinux { +// Owned via std::shared_ptr by its creator (never std::unique_ptr). EnqueueTask +// schedules a GLib idle callback that runs asynchronously on the main loop, and +// enable_shared_from_this lets that callback hold a weak reference so it can +// detect the runner having been destroyed in the meantime (for example +// stopAudioRenderer tearing down the sink while a callback is still queued) +// instead of dereferencing freed memory. +class TaskRunnerLinux : public std::enable_shared_from_this { public: TaskRunnerLinux() = default; ~TaskRunnerLinux() = default;