Fix a handle + memory + whatever else leak when automatically changing tracks in Windows. Fixes issue #782

src/CMakeLists.txt

@@ -372,8 +372,16 @@ else (LINGUAS STREQUAL "All")
 endif (LINGUAS STREQUAL "All")
 
 # Engines
-set(GST_ENGINE_SRC engines/gstengine.cpp engines/gstenginepipeline.cpp)
-set(GST_ENGINE_MOC engines/gstengine.h engines/gstenginepipeline.h)
+set(GST_ENGINE_SRC
+  engines/gstengine.cpp
+  engines/gstenginepipeline.cpp
+  engines/gstelementdeleter.cpp
+)
+set(GST_ENGINE_MOC
+  engines/gstengine.h
+  engines/gstenginepipeline.h
+  engines/gstelementdeleter.h
+)
 set(GST_ENGINE_LIB GSTREAMER GSTREAMER_BASE)
 set(XINE_ENGINE_SRC engines/xine-engine.cpp engines/xine-scope.c)
 set(XINE_ENGINE_MOC engines/xine-engine.h)

src/engines/gstelementdeleter.cpp

@@ -0,0 +1,33 @@
+/* This file is part of Clementine.
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "gstelementdeleter.h"
+
+#include <QtDebug>
+
+GstElementDeleter::GstElementDeleter(QObject* parent)
+  : QObject(parent)
+{
+}
+
+void GstElementDeleter::DeleteElementLater(GstElement* element) {
+  metaObject()->invokeMethod(this, "DeleteElement", Qt::QueuedConnection,
+                             Q_ARG(GstElement*, element));
+}
+
+void GstElementDeleter::DeleteElement(GstElement* element) {
+  gst_element_set_state(element, GST_STATE_NULL);
+}

src/engines/gstelementdeleter.h

@@ -0,0 +1,42 @@
+/* This file is part of Clementine.
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef GSTBINDELETER_H
+#define GSTBINDELETER_H
+
+#include <QObject>
+
+#include <gst/gst.h>
+
+class GstElementDeleter : public QObject {
+  Q_OBJECT
+
+public:
+  GstElementDeleter(QObject* parent = 0);
+
+  // If you call this function with any gstreamer element, the element will get
+  // deleted in the main thread.  This is useful if you need to delete an
+  // element from its own callback.
+  // It's in a separate object so *your* object (GstEnginePipeline) can be
+  // destroyed, and the element that you scheduled for deletion is still
+  // deleted later regardless.
+  void DeleteElementLater(GstElement* element);
+
+private slots:
+  void DeleteElement(GstElement* element);
+};
+
+#endif // GSTBINDELETER_H

src/engines/gstenginepipeline.cpp

@@ -16,6 +16,7 @@
 
 #include <limits>
 
+#include "gstelementdeleter.h"
 #include "gstenginepipeline.h"
 #include "gstengine.h"
 #include "bufferconsumer.h"
@@ -30,6 +31,9 @@ const int GstEnginePipeline::kEqBandCount = 10;
 const int GstEnginePipeline::kEqBandFrequencies[] = {
   60, 170, 310, 600, 1000, 3000, 6000, 12000, 14000, 16000};
 
+GstElementDeleter* GstEnginePipeline::sElementDeleter = NULL;
+
+
 GstEnginePipeline::GstEnginePipeline(GstEngine* engine)
   : QObject(NULL),
     engine_(engine),
@@ -59,6 +63,10 @@ GstEnginePipeline::GstEnginePipeline(GstEngine* engine)
     audioscale_(NULL),
     audiosink_(NULL)
 {
+  if (!sElementDeleter) {
+    sElementDeleter = new GstElementDeleter;
+  }
+
   for (int i=0 ; i<kEqBandCount ; ++i)
     eq_band_gains_ << 0;
 }
@@ -76,11 +84,6 @@ void GstEnginePipeline::set_replaygain(bool enabled, int mode, float preamp,
   rg_compression_ = compression;
 }
 
-bool GstEnginePipeline::StopUriDecodeBin(gpointer bin) {
-  gst_element_set_state(GST_ELEMENT(bin), GST_STATE_NULL);
-  return false; // So it doesn't get called again
-}
-
 bool GstEnginePipeline::ReplaceDecodeBin(GstElement* new_bin) {
   if (!new_bin) return false;
 
@@ -88,8 +91,8 @@ bool GstEnginePipeline::ReplaceDecodeBin(GstElement* new_bin) {
   if (uridecodebin_) {
     gst_bin_remove(GST_BIN(pipeline_), uridecodebin_);
 
-    // Note that the caller to this function MUST schedule StopUriDecodeBin in
-    // the main thread on the old bin.
+    // Note that the caller to this function MUST schedule the old bin for
+    // deletion in the main thread
   }
 
   uridecodebin_ = new_bin;
@@ -426,7 +429,7 @@ void GstEnginePipeline::SourceDrainedCallback(GstURIDecodeBin* bin, gpointer sel
 
     // This has to happen *after* the gst_element_set_state on the new bin to
     // fix an occasional race condition deadlock.
-    g_idle_add(GSourceFunc(StopUriDecodeBin), old_decode_bin);
+    sElementDeleter->DeleteElementLater(old_decode_bin);
 
     instance->ignore_tags_ = false;
   }

src/engines/gstenginepipeline.h

@@ -29,6 +29,7 @@
 
 #include "engine_fwd.h"
 
+class GstElementDeleter;
 class GstEngine;
 class BufferConsumer;
 
@@ -99,7 +100,6 @@ class GstEnginePipeline : public QObject {
   static bool HandoffCallback(GstPad*, GstBuffer*, gpointer);
   static bool EventHandoffCallback(GstPad*, GstEvent*, gpointer);
   static void SourceDrainedCallback(GstURIDecodeBin*, gpointer);
-  static bool StopUriDecodeBin(gpointer bin);
   void TagMessageReceived(GstMessage*);
   void ErrorMessageReceived(GstMessage*);
   void ElementMessageReceived(GstMessage*);
@@ -122,6 +122,8 @@ class GstEnginePipeline : public QObject {
   static const int kEqBandCount;
   static const int kEqBandFrequencies[];
 
+  static GstElementDeleter* sElementDeleter;
+
   GstEngine* engine_;
 
   // General settings for the pipeline

src/main.cpp

@@ -54,7 +54,7 @@
 #endif
 
 #ifdef HAVE_GSTREAMER
-#  include <gst/gstbuffer.h>
+#  include <gst/gst.h>
    class GstEnginePipeline;
 #endif
 
@@ -125,6 +125,7 @@ int main(int argc, char *argv[]) {
 
 #ifdef HAVE_GSTREAMER
   qRegisterMetaType<GstBuffer*>("GstBuffer*");
+  qRegisterMetaType<GstElement*>("GstElement*");
   qRegisterMetaType<GstEnginePipeline*>("GstEnginePipeline*");
 #endif