Fix MoodbarPipeline crash on gstreamer error.

As reported in issue 6302, playing a stream that causes gstreamer to error at
start can cause a crash. The problem occurs when the MoodbarPipeline receives a
pad-added signal after it has handled an error callback. In the error callback,
the builder_ is freed. In the pad-added handler (NewPadCallback), this object
is accessed.

This change adds a running_ flag that we is set when the pipeline is started and
cleared on an error. We check this flag at the beginning of NewPadCallback. For
sanity sake, we also check the builder_ pointer before dereferencing.

This solution is not complete as there still some syncronization issues. With
this specific situation, the error and new pad callbacks appear to always occur
on the same thread, but it's probably not true for all error conditions. The
object is also destroyed by a different thread, so it's possible that an async
callback may occur at the wrong time during the deletion of the object.

See clementine-player#6302

src/moodbar/moodbarpipeline.cpp

@@ -37,7 +37,8 @@ MoodbarPipeline::MoodbarPipeline(const QUrl& local_filename)
       local_filename_(local_filename),
       pipeline_(nullptr),
       convert_element_(nullptr),
-      success_(false) {}
+      success_(false),
+      running_(false) {}
 
 MoodbarPipeline::~MoodbarPipeline() { Cleanup(); }
 
@@ -117,6 +118,7 @@ void MoodbarPipeline::Start() {
   gst_object_unref(bus);
 
   // Start playing
+  running_ = true;
   gst_element_set_state(pipeline_, GST_STATE_PLAYING);
 }
 
@@ -135,6 +137,12 @@ void MoodbarPipeline::ReportError(GstMessage* msg) {
 
 void MoodbarPipeline::NewPadCallback(GstElement*, GstPad* pad, gpointer data) {
   MoodbarPipeline* self = reinterpret_cast<MoodbarPipeline*>(data);
+
+  if (!self->running_) {
+    qLog(Warning) << "Received gstreamer callback after pipeline has stopped.";
+    return;
+  }
+
   GstPad* const audiopad =
       gst_element_get_static_pad(self->convert_element_, "sink");
 
@@ -152,7 +160,10 @@ void MoodbarPipeline::NewPadCallback(GstElement*, GstPad* pad, gpointer data) {
   gst_structure_get_int(structure, "rate", &rate);
   gst_caps_unref(caps);
 
-  self->builder_->Init(kBands, rate);
+  if (self->builder_ != nullptr)
+    self->builder_->Init(kBands, rate);
+  else
+    qLog(Error) << "Builder does not exist";
 }
 
 GstBusSyncReply MoodbarPipeline::BusCallbackSync(GstBus*, GstMessage* msg,
@@ -177,6 +188,7 @@ GstBusSyncReply MoodbarPipeline::BusCallbackSync(GstBus*, GstMessage* msg,
 
 void MoodbarPipeline::Stop(bool success) {
   success_ = success;
+  running_ = false;
   if (builder_ != nullptr) {
     data_ = builder_->Finish(1000);
     builder_.reset();
@@ -189,6 +201,7 @@ void MoodbarPipeline::Cleanup() {
   Q_ASSERT(QThread::currentThread() == thread());
   Q_ASSERT(QThread::currentThread() != qApp->thread());
 
+  running_ = false;
   if (pipeline_) {
     GstBus* bus = gst_pipeline_get_bus(GST_PIPELINE(pipeline_));
     gst_bus_set_sync_handler(bus, nullptr, nullptr, nullptr);

src/moodbar/moodbarpipeline.h

@@ -71,6 +71,7 @@ class MoodbarPipeline : public QObject {
   std::unique_ptr<MoodbarBuilder> builder_;
 
   bool success_;
+  bool running_;
   QByteArray data_;
 };