From b9b70399d8fd04813aa26db4ac9fd46c95c7909e Mon Sep 17 00:00:00 2001
From: Jonas Kvinge <jonas@jkvinge.net>
Date: Sat, 20 Dec 2025 01:28:53 +0100
Subject: [PATCH] GstEnginePipeline: Fix possible race condition in pipeline
 destructor

Wait for ongoing state changes to complete before setting pipeline to NULL.
This prevents race conditions with async state transitions that can cause crashes in GStreamer elements.

Fixes #1875
---
 src/engine/gstenginepipeline.cpp | 30 ++++++++++++++++++++++++++++++
 src/engine/gstenginepipeline.h   |  4 ++++
 2 files changed, 34 insertions(+)

diff --git a/src/engine/gstenginepipeline.cpp b/src/engine/gstenginepipeline.cpp
index 5c79d9c3e..5cdb3807a 100644
--- a/src/engine/gstenginepipeline.cpp
+++ b/src/engine/gstenginepipeline.cpp
@@ -26,6 +26,7 @@
 #include <cstdint>
 #include <cstring>
 #include <cmath>
+#include <algorithm>
 
 #include <glib.h>
 #include <glib-object.h>
@@ -215,6 +216,23 @@ GstEnginePipeline::~GstEnginePipeline() {
 
   if (pipeline_) {
 
+    // Wait for any ongoing state changes for this pipeline to complete before setting to NULL.
+    // This prevents race conditions with async state transitions.
+    {
+      // Copy futures to local list to avoid holding mutex during waitForFinished()
+      QList<QFuture<GstStateChangeReturn>> futures_to_wait;
+      {
+        QMutexLocker locker(&mutex_pending_state_changes_);
+        futures_to_wait = pending_state_changes_;
+        pending_state_changes_.clear();
+      }
+
+      // Wait for all pending futures to complete
+      for (QFuture<GstStateChangeReturn> &future : futures_to_wait) {
+        future.waitForFinished();
+      }
+    }
+
     gst_element_set_state(pipeline_, GST_STATE_NULL);
 
     GstElement *audiobin = nullptr;
@@ -1869,6 +1887,12 @@ QFuture<GstStateChangeReturn> GstEnginePipeline::SetState(const GstState state)
   QFuture<GstStateChangeReturn> future = QtConcurrent::run(shared_state_threadpool(), &gst_element_set_state, pipeline_, state);
   watcher->setFuture(future);
 
+  // Track this future so destructor can wait for it
+  {
+    QMutexLocker locker(&mutex_pending_state_changes_);
+    pending_state_changes_.append(future);
+  }
+
   return future;
 
 }
@@ -1878,6 +1902,12 @@ void GstEnginePipeline::SetStateFinishedSlot(const GstState state, const GstStat
   last_set_state_in_progress_ = GST_STATE_VOID_PENDING;
   --set_state_in_progress_;
 
+  // Remove finished futures from tracking list to prevent unbounded growth
+  {
+    QMutexLocker locker(&mutex_pending_state_changes_);
+    pending_state_changes_.erase(std::remove_if(pending_state_changes_.begin(), pending_state_changes_.end(), [](const QFuture<GstStateChangeReturn> &f) { return f.isFinished(); }), pending_state_changes_.end());
+  }
+
   switch (state_change_return) {
     case GST_STATE_CHANGE_SUCCESS:
     case GST_STATE_CHANGE_ASYNC:
diff --git a/src/engine/gstenginepipeline.h b/src/engine/gstenginepipeline.h
index 1af9f2d7b..7fe6a6e0f 100644
--- a/src/engine/gstenginepipeline.h
+++ b/src/engine/gstenginepipeline.h
@@ -385,6 +385,10 @@ class GstEnginePipeline : public QObject {
 
   mutex_protected<GstState> last_set_state_in_progress_;
   mutex_protected<GstState> last_set_state_async_in_progress_;
+
+  // Track futures for this pipeline's state changes to allow waiting for them in destructor
+  QList<QFuture<GstStateChangeReturn>> pending_state_changes_;
+  QMutex mutex_pending_state_changes_;
 };
 
 using GstEnginePipelinePtr = QSharedPointer<GstEnginePipeline>;