Fix crashes on x-thread signal emission.

Fix crashes on x-thread signal emission if the emitting object
is destroyed between the storage of the message on the queue
and the emission of the object in the UI thread.

14 files changed, 175 insertions, 49 deletions

diff --git a/src/lib/content.cc b/src/lib/content.cc
index fcc658717..b00ea3e57 100644
--- a/src/lib/content.cc
+++ b/src/lib/content.cc
@@ -153,9 +153,7 @@ Content::examine (shared_ptr<Job> job)
 void
 Content::signal_changed (int p)
 {
-	if (ui_signaller) {
-		ui_signaller->emit (boost::bind (boost::ref (Changed), shared_from_this (), p, _change_signals_frequent));
-	}
+	emit (boost::bind (boost::ref (Changed), shared_from_this (), p, _change_signals_frequent));
 }
 
 void
diff --git a/src/lib/content.h b/src/lib/content.h
index c6cede5fa..2b966110b 100644
--- a/src/lib/content.h
+++ b/src/lib/content.h
@@ -25,6 +25,7 @@
 #define DCPOMATIC_CONTENT_H
 
 #include "types.h"
+#include "signaller.h"
 #include "dcpomatic_time.h"
 #include <libxml++/libxml++.h>
 #include <libcxml/cxml.h>
@@ -53,7 +54,7 @@ public:
 /** @class Content
  *  @brief A piece of content represented by one or more files on disk.
  */
-class Content : public boost::enable_shared_from_this<Content>, public boost::noncopyable
+class Content : public boost::enable_shared_from_this<Content>, public Signaller, public boost::noncopyable
 {
 public:
 	Content (boost::shared_ptr<const Film>);
diff --git a/src/lib/film.cc b/src/lib/film.cc
index 80755a4cb..35773c797 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -32,7 +32,6 @@
 #include "exceptions.h"
 #include "examine_content_job.h"
 #include "config.h"
-#include "ui_signaller.h"
 #include "playlist.h"
 #include "player.h"
 #include "dcp_content_type.h"
@@ -790,9 +789,7 @@ Film::signal_changed (Property p)
 		break;
 	}
 
-	if (ui_signaller) {
-		ui_signaller->emit (boost::bind (boost::ref (Changed), p));
-	}
+	emit (boost::bind (boost::ref (Changed), p));
 }
 
 void
@@ -995,9 +992,7 @@ Film::playlist_content_changed (boost::weak_ptr<Content> c, int p)
 		signal_changed (NAME);
 	}
 
-	if (ui_signaller) {
-		ui_signaller->emit (boost::bind (boost::ref (ContentChanged), c, p));
-	}
+	emit (boost::bind (boost::ref (ContentChanged), c, p));
 }
 
 void
diff --git a/src/lib/film.h b/src/lib/film.h
index 3cd370a0d..f61062be0 100644
--- a/src/lib/film.h
+++ b/src/lib/film.h
@@ -29,6 +29,7 @@
 #include "types.h"
 #include "isdcf_metadata.h"
 #include "frame_rate_change.h"
+#include "signaller.h"
 #include "ratio.h"
 #include <dcp/key.h>
 #include <dcp/encrypted_kdm.h>
@@ -55,7 +56,7 @@ struct isdcf_name_test;
  *
  *  The content of a Film is held in a Playlist (created and managed by the Film).
  */
-class Film : public boost::enable_shared_from_this<Film>, public boost::noncopyable
+class Film : public boost::enable_shared_from_this<Film>, public Signaller, public boost::noncopyable
 {
 public:
 	Film (boost::filesystem::path, bool log = true);
diff --git a/src/lib/job.cc b/src/lib/job.cc
index eadafbf73..f28146632 100644
--- a/src/lib/job.cc
+++ b/src/lib/job.cc
@@ -203,8 +203,8 @@ Job::set_state (State s)
 		}
 	}
 
-	if (finished && ui_signaller) {
-		ui_signaller->emit (boost::bind (boost::ref (Finished)));
+	if (finished) {
+		emit (boost::bind (boost::ref (Finished)));
 	}	
 }
 
@@ -239,9 +239,7 @@ Job::set_progress (float p, bool force)
 		_pause_changed.wait (lm2);
 	}
 
-	if (ui_signaller) {
-		ui_signaller->emit (boost::bind (boost::ref (Progress)));
-	}
+	emit (boost::bind (boost::ref (Progress)));
 }
 
 /** @return fractional progress of the current sub-job, if known */
@@ -301,9 +299,7 @@ Job::set_progress_unknown ()
 	_progress.reset ();
 	lm.unlock ();
 
-	if (ui_signaller) {
-		ui_signaller->emit (boost::bind (boost::ref (Progress)));
-	}
+	emit (boost::bind (boost::ref (Progress)));
 }
 
 /** @return Human-readable status of this job */
diff --git a/src/lib/job.h b/src/lib/job.h
index 7c6707880..8fe87747c 100644
--- a/src/lib/job.h
+++ b/src/lib/job.h
@@ -24,6 +24,7 @@
 #ifndef DCPOMATIC_JOB_H
 #define DCPOMATIC_JOB_H
 
+#include "signaller.h"
 #include <boost/thread/mutex.hpp>
 #include <boost/enable_shared_from_this.hpp>
 #include <boost/signals2.hpp>
@@ -35,7 +36,7 @@ class Film;
 /** @class Job
  *  @brief A parent class to represent long-running tasks which are run in their own thread.
  */
-class Job : public boost::enable_shared_from_this<Job>, public boost::noncopyable
+class Job : public boost::enable_shared_from_this<Job>, public Signaller, public boost::noncopyable
 {
 public:
 	Job (boost::shared_ptr<const Film>);
diff --git a/src/lib/job_manager.cc b/src/lib/job_manager.cc
index 2b727b0aa..63db662d0 100644
--- a/src/lib/job_manager.cc
+++ b/src/lib/job_manager.cc
@@ -64,9 +64,7 @@ JobManager::add (shared_ptr<Job> j)
 		_jobs.push_back (j);
 	}
 
-	if (ui_signaller) {
-		ui_signaller->emit (boost::bind (boost::ref (JobAdded), weak_ptr<Job> (j)));
-	}
+	emit (boost::bind (boost::ref (JobAdded), weak_ptr<Job> (j)));
 	
 	return j;
 }
@@ -138,9 +136,7 @@ JobManager::scheduler ()
 
 		if (active_jobs != _last_active_jobs) {
 			_last_active_jobs = active_jobs;
-			if (ui_signaller) {
-				ui_signaller->emit (boost::bind (boost::ref (ActiveJobsChanged), active_jobs));
-			}
+			emit (boost::bind (boost::ref (ActiveJobsChanged), active_jobs));
 		}
 
 		dcpomatic_sleep (1);
diff --git a/src/lib/job_manager.h b/src/lib/job_manager.h
index 9d8620cbb..b946c1a98 100644
--- a/src/lib/job_manager.h
+++ b/src/lib/job_manager.h
@@ -21,6 +21,7 @@
  *  @brief A simple scheduler for jobs.
  */
 
+#include "signaller.h"
 #include <boost/thread/mutex.hpp>
 #include <boost/thread.hpp>
 #include <boost/signals2.hpp>
@@ -32,7 +33,7 @@ extern void wait_for_jobs ();
 /** @class JobManager
  *  @brief A simple scheduler for jobs.
  */
-class JobManager : public boost::noncopyable
+class JobManager : public Signaller, public boost::noncopyable
 {
 public:
 
diff --git a/src/lib/server_finder.cc b/src/lib/server_finder.cc
index 979046dab..72a9a4ef5 100644
--- a/src/lib/server_finder.cc
+++ b/src/lib/server_finder.cc
@@ -173,7 +173,7 @@ ServerFinder::handle_accept (boost::system::error_code ec, shared_ptr<Socket> so
 			boost::mutex::scoped_lock lm (_mutex);
 			_servers.push_back (sd);
 		}
-		ui_signaller->emit (boost::bind (boost::ref (ServerFound), sd));
+		emit (boost::bind (boost::ref (ServerFound), sd));
 	}
 
 	start_accept ();
diff --git a/src/lib/server_finder.h b/src/lib/server_finder.h
index 3fab6864a..dc62f998d 100644
--- a/src/lib/server_finder.h
+++ b/src/lib/server_finder.h
@@ -18,9 +18,10 @@
 */
 
 #include "server.h"
+#include "signaller.h"
 #include <boost/signals2.hpp>
 
-class ServerFinder : public ExceptionStore
+class ServerFinder : public Signaller, public ExceptionStore
 {
 public:
 	boost::signals2::connection connect (boost::function<void (ServerDescription)>);
diff --git a/src/lib/signaller.h b/src/lib/signaller.h
new file mode 100644
index 000000000..408cfcf5b
--- /dev/null
+++ b/src/lib/signaller.h
@@ -0,0 +1,131 @@
+/*
+    Copyright (C) 2015 Carl Hetherington <cth@carlh.net>
+
+    This program 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 2 of the License, or
+    (at your option) any later version.
+
+    This program 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 this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+*/
+
+#ifndef DCPOMATIC_SIGNALLER_H
+#define DCPOMATIC_SIGNALLER_H
+
+#include "ui_signaller.h"
+#include <boost/thread/mutex.hpp>
+#include <boost/signals2.hpp>
+
+class WrapperBase
+{
+public:
+	WrapperBase ()
+		: _valid (true)
+		, _finished (false)
+	{}
+
+	virtual ~WrapperBase () {}
+
+	/* Can be called from any thread */
+	void invalidate ()
+	{
+		boost::mutex::scoped_lock lm (_mutex);
+		_valid = false;
+	}
+
+	bool finished () const {
+		boost::mutex::scoped_lock lm (_mutex);
+		return _finished;
+	}
+
+protected:
+	/* Protect _valid and _finished */
+	mutable boost::mutex _mutex;
+	bool _valid;
+	bool _finished;
+};
+
+/** Helper class to manage lifetime of signals, specifically to address
+ *  the problem where an object containing a signal is deleted before
+ *  its signal is emitted.
+ */
+template <class T>
+class Wrapper : public WrapperBase
+{
+public:
+	Wrapper (T signal)
+		: _signal (signal)
+	{
+
+	}
+
+	/* Called by the UI thread only */
+	void signal ()
+	{
+		boost::mutex::scoped_lock lm (_mutex);
+		if (_valid) {
+			_signal ();
+		}
+		_finished = true;
+	}
+
+private:
+	T _signal;
+};
+
+/** Parent for any class which needs to raise cross-thread signals (from non-UI
+ *  to UI).  Subclasses should call, e.g. emit (boost::bind (boost::ref (MySignal), foo, bar));
+ */
+class Signaller
+{
+public:
+	/* Can be called from any thread */
+	virtual ~Signaller () {
+		boost::mutex::scoped_lock lm (_mutex);
+		for (std::list<WrapperBase*>::iterator i = _wrappers.begin(); i != _wrappers.end(); ++i) {
+			(*i)->invalidate ();
+		}
+	}
+
+	/* Can be called from any thread */
+	template <class T>
+	void emit (T signal)
+	{
+		Wrapper<T>* w = new Wrapper<T> (signal);
+		if (ui_signaller) {
+			ui_signaller->emit (boost::bind (&Wrapper<T>::signal, w));
+		}
+		
+		boost::mutex::scoped_lock lm (_mutex);
+
+		/* Clean up finished Wrappers */
+		std::list<WrapperBase*>::iterator i = _wrappers.begin ();
+		while (i != _wrappers.end ()) {
+			std::list<WrapperBase*>::iterator tmp = i;
+			++tmp;
+			if ((*i)->finished ()) {
+				delete *i;
+				_wrappers.erase (i);
+			}
+			i = tmp;
+		}
+
+		/* Add the new one */
+		_wrappers.push_back (w);
+	}
+
+private:
+	/* Protect _wrappers */
+	boost::mutex _mutex;
+	std::list<WrapperBase*> _wrappers;
+};
+
+#endif
diff --git a/src/lib/ui_signaller.h b/src/lib/ui_signaller.h
index ee4d230d4..9d4495cd1 100644
--- a/src/lib/ui_signaller.h
+++ b/src/lib/ui_signaller.h
@@ -24,6 +24,8 @@
 #include <boost/asio.hpp>
 #include <boost/thread.hpp>
 
+class Signaller;
+
 /** A class to allow signals to be emitted from non-UI threads and handled
  *  by a UI thread.
  */
@@ -37,23 +39,6 @@ public:
 		_ui_thread = boost::this_thread::get_id ();
 	}
 
-	/** Emit a signal from any thread whose handlers will be called in the UI
-	 *  thread.  Use something like:
-	 *
-	 *  ui_signaller->emit (boost::bind (boost::ref (SomeSignal), parameter));
-	 */
-	template <typename T>
-	void emit (T f) {
-		if (boost::this_thread::get_id() == _ui_thread) {
-			/* already in the UI thread */
-			f ();
-		} else {
-			/* non-UI thread; post to the service and wake up the UI */
-			_service.post (f);
-			wake_ui ();
-		}
-	}
-
 	/* Do something next time the UI is idle */
 	template <typename T>
 	void when_idle (T f) {
@@ -73,6 +58,25 @@ public:
 	}
 
 private:
+	/** Emit a signal from any thread whose handlers will be called in the UI
+	 *  thread.  Use something like:
+	 *
+	 *  ui_signaller->emit (boost::bind (boost::ref (SomeSignal), parameter));
+	 */
+	template <typename T>
+	void emit (T f) {
+		if (boost::this_thread::get_id() == _ui_thread) {
+			/* already in the UI thread */
+			f ();
+		} else {
+			/* non-UI thread; post to the service and wake up the UI */
+			_service.post (f);
+			wake_ui ();
+		}
+	}
+
+	friend class Signaller;
+	
 	/** A io_service which is used as the conduit for messages */
 	boost::asio::io_service _service;
 	/** Object required to keep io_service from stopping when it has nothing to do */
diff --git a/src/lib/update.cc b/src/lib/update.cc
index a05df8ef3..26944ecc3 100644
--- a/src/lib/update.cc
+++ b/src/lib/update.cc
@@ -168,7 +168,7 @@ UpdateChecker::set_state (State s)
 		_emits++;
 	}
 
-	ui_signaller->emit (boost::bind (boost::ref (StateChanged)));
+	emit (boost::bind (boost::ref (StateChanged)));
 }
 
 UpdateChecker *
diff --git a/src/lib/update.h b/src/lib/update.h
index 5bb9e9501..461217a37 100644
--- a/src/lib/update.h
+++ b/src/lib/update.h
@@ -21,6 +21,7 @@
  *  @brief UpdateChecker class.
  */
 
+#include "signaller.h"
 #include <curl/curl.h>
 #include <boost/signals2.hpp>
 #include <boost/thread/mutex.hpp>
@@ -30,7 +31,7 @@
 struct update_checker_test;
 
 /** Class to check for the existance of an update for DCP-o-matic on a remote server */
-class UpdateChecker : public boost::noncopyable
+class UpdateChecker : public Signaller, public boost::noncopyable
 {
 public:
 	UpdateChecker ();