Use a new UISignal which checks thread safety slightly.3052-hang

This adds a wrapper around signals2::signal which checks that emission
happens from the GUI thread, for signals whose handlers must be called
in the UI thread.  I'm not sure how helpful it really is but maybe it
catches some bad situations.

13 files changed, 117 insertions, 41 deletions

diff --git a/src/lib/config.h b/src/lib/config.h
index c90790ebc..0fbc3b7e9 100644
--- a/src/lib/config.h
+++ b/src/lib/config.h
@@ -31,6 +31,7 @@
 #include "enum_indexed_vector.h"
 #include "export_config.h"
 #include "rough_duration.h"
+#include "signal.h"
 #include "state.h"
 #include "video_encoding.h"
 #include <dcp/name_format.h>
@@ -1250,7 +1251,7 @@ public:
 
 
 	void changed(Property p = OTHER);
-	boost::signals2::signal<void (Property)> Changed;
+	UISignal<void (Property)> Changed;
 	/** Emitted if read() failed on an existing Config file.  There is nothing
 	    a listener can do about it: this is just for information.
 	*/
diff --git a/src/lib/encode_server_finder.cc b/src/lib/encode_server_finder.cc
index 1a0329dd7..1bee8eb52 100644
--- a/src/lib/encode_server_finder.cc
+++ b/src/lib/encode_server_finder.cc
@@ -169,7 +169,7 @@ try
 		}
 
 		if (removed) {
-			emit (boost::bind(boost::ref(ServersListChanged)));
+			ServersListChanged.emit_ui(this);
 		}
 
 		boost::mutex::scoped_lock lm (_search_condition_mutex);
@@ -269,7 +269,7 @@ EncodeServerFinder::handle_accept (boost::system::error_code ec)
 	}
 
 	if (changed) {
-		emit (boost::bind(boost::ref (ServersListChanged)));
+		ServersListChanged.emit_ui(this);
 	}
 
 	start_accept ();
diff --git a/src/lib/encode_server_finder.h b/src/lib/encode_server_finder.h
index 722786b77..d5db0234d 100644
--- a/src/lib/encode_server_finder.h
+++ b/src/lib/encode_server_finder.h
@@ -27,6 +27,7 @@
 #include "config.h"
 #include "encode_server_description.h"
 #include "exception_store.h"
+#include "signal.h"
 #include "signaller.h"
 #include <boost/signals2.hpp>
 #include <boost/thread/condition.hpp>
@@ -53,7 +54,7 @@ public:
 	std::list<EncodeServerDescription> servers () const;
 
 	/** Emitted whenever the list of servers changes */
-	boost::signals2::signal<void ()> ServersListChanged;
+	UISignal<void ()> ServersListChanged;
 
 private:
 	EncodeServerFinder ();
diff --git a/src/lib/film.cc b/src/lib/film.cc
index 09378696e..d7cb05930 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -1310,7 +1310,7 @@ Film::signal_change(ChangeType type, FilmProperty p)
 	if (type == ChangeType::DONE) {
 		set_dirty(true);
 
-		emit(boost::bind(boost::ref(Change), type, p));
+		Change.emit_ui(this, type, p);
 
 		if (p == FilmProperty::VIDEO_FRAME_RATE || p == FilmProperty::SEQUENCE) {
 			/* We want to call Playlist::maybe_sequence but this must happen after the
@@ -1577,7 +1577,7 @@ Film::playlist_content_change(ChangeType type, weak_ptr<Content> c, int p, bool
 	}
 
 	if (type == ChangeType::DONE) {
-		emit(boost::bind(boost::ref(ContentChange), type, c, p, frequent));
+		ContentChange.emit_ui(this, type, c, p, frequent);
 		if (!frequent) {
 			check_settings_consistency();
 		}
@@ -2325,7 +2325,7 @@ Film::set_dirty(bool dirty)
 	auto const changed = dirty != _dirty;
 	_dirty = dirty;
 	if (changed) {
-		emit(boost::bind(boost::ref(DirtyChange), _dirty));
+		DirtyChange.emit_ui(this, _dirty);
 	}
 }
 
diff --git a/src/lib/film.h b/src/lib/film.h
index c14b0f4fa..9f066e628 100644
--- a/src/lib/film.h
+++ b/src/lib/film.h
@@ -38,6 +38,7 @@
 #include "named_channel.h"
 #include "remembered_asset.h"
 #include "resolution.h"
+#include "signal.h"
 #include "signaller.h"
 #include "territory_type.h"
 #include "transcode_job.h"
@@ -440,20 +441,20 @@ public:
 	boost::filesystem::path info_file(dcpomatic::DCPTimePeriod p) const;
 
 	/** Emitted when some property has of the Film is about to change or has changed */
-	mutable boost::signals2::signal<void (ChangeType, FilmProperty)> Change;
+	mutable UISignal<void (ChangeType, FilmProperty)> Change;
 
 	/** Emitted when some property of our content has changed */
-	mutable boost::signals2::signal<void (ChangeType, std::weak_ptr<Content>, int, bool)> ContentChange;
+	mutable UISignal<void (ChangeType, std::weak_ptr<Content>, int, bool)> ContentChange;
 
 	/** Emitted when the film's length might have changed; this is not like a normal
 	    property as its value is derived from the playlist, so it has its own signal.
 	*/
-	mutable boost::signals2::signal<void ()> LengthChange;
+	mutable UISignal<void ()> LengthChange;
 
-	boost::signals2::signal<void (bool)> DirtyChange;
+	UISignal<void (bool)> DirtyChange;
 
 	/** Emitted when we have something important to tell the user */
-	boost::signals2::signal<void (std::string)> Message;
+	UISignal<void (std::string)> Message;
 
 	/** Current version number of the state file */
 	static int const current_state_version;
diff --git a/src/lib/hints.cc b/src/lib/hints.cc
index 17aa16d47..5e9756b2b 100644
--- a/src/lib/hints.cc
+++ b/src/lib/hints.cc
@@ -411,11 +411,11 @@ Hints::scan_content(shared_ptr<const Film> film)
 	}
 
 	if (check_loudness_done && have_text) {
-		emit(boost::bind(boost::ref(Progress), _("Examining subtitles and closed captions")));
+		Progress.emit_ui(this, _("Examining subtitles and closed captions"));
 	} else if (!check_loudness_done && !have_text) {
-		emit(boost::bind(boost::ref(Progress), _("Examining audio")));
+		Progress.emit_ui(this, _("Examining audio"));
 	} else {
-		emit(boost::bind(boost::ref(Progress), _("Examining audio, subtitles and closed captions")));
+		Progress.emit_ui(this, _("Examining audio, subtitles and closed captions"));
 	}
 
 	auto player = make_shared<Player>(film, Image::Alignment::COMPACT, false);
@@ -442,7 +442,7 @@ Hints::scan_content(shared_ptr<const Film> film)
 			if (_stop) {
 				return;
 			}
-			emit(boost::bind(boost::ref(Pulse)));
+			Pulse.emit_ui(this);
 			last_pulse = now;
 		}
 	}
@@ -536,7 +536,7 @@ try
 	}
 	dcp::filesystem::remove_all(dcp_dir);
 
-	emit(boost::bind(boost::ref(Finished)));
+	Finished.emit_ui(this);
 }
 catch (boost::thread_interrupted)
 {
@@ -545,14 +545,14 @@ catch (boost::thread_interrupted)
 catch (...)
 {
 	store_current();
-	emit(boost::bind(boost::ref(Finished)));
+	Finished.emit_ui(this);
 }
 
 
 void
 Hints::hint(string h)
 {
-	emit(boost::bind(boost::ref(Hint), h));
+	Hint.emit_ui(this, h);
 }
 
 
diff --git a/src/lib/hints.h b/src/lib/hints.h
index 49fd32ea9..56a5eb787 100644
--- a/src/lib/hints.h
+++ b/src/lib/hints.h
@@ -23,11 +23,11 @@
 #include "dcp_text_track.h"
 #include "dcpomatic_time.h"
 #include "player_text.h"
+#include "signal.h"
 #include "signaller.h"
 #include "text_type.h"
 #include "weak_film.h"
 #include <boost/atomic.hpp>
-#include <boost/signals2.hpp>
 #include <string>
 #include <vector>
 
@@ -44,10 +44,10 @@ public:
 
 	void start();
 
-	boost::signals2::signal<void (std::string)> Hint;
-	boost::signals2::signal<void (std::string)> Progress;
-	boost::signals2::signal<void (void)> Pulse;
-	boost::signals2::signal<void (void)> Finished;
+	UISignal<void (std::string)> Hint;
+	UISignal<void (std::string)> Progress;
+	UISignal<void ()> Pulse;
+	UISignal<void ()> Finished;
 
 	/* For tests only */
 	void join();
diff --git a/src/lib/job.cc b/src/lib/job.cc
index ee6ad4e70..9f28c2bc2 100644
--- a/src/lib/job.cc
+++ b/src/lib/job.cc
@@ -375,7 +375,7 @@ Job::set_state (State s)
 
 	if (finished) {
 		auto const result = state_to_result(s);
-		emit(boost::bind(boost::ref(Finished), result));
+		Finished.emit_ui(this, result);
 		FinishedImmediate(result);
 	}
 }
@@ -419,7 +419,7 @@ Job::check_for_interruption_or_pause ()
 
 	boost::mutex::scoped_lock lm (_state_mutex);
 	while (_state == PAUSED_BY_USER || _state == PAUSED_BY_PRIORITY) {
-		emit (boost::bind (boost::ref (Progress)));
+		Progress.emit_ui(this);
 		_pause_changed.wait (lm);
 	}
 }
@@ -475,7 +475,7 @@ Job::set_progress_common (optional<float> p)
 		_progress = p;
 	}
 
-	emit (boost::bind (boost::ref (Progress)));
+	Progress.emit_ui(this);
 }
 
 
diff --git a/src/lib/job.h b/src/lib/job.h
index 9b5fdfa6e..a80d66cda 100644
--- a/src/lib/job.h
+++ b/src/lib/job.h
@@ -28,6 +28,7 @@
 #define DCPOMATIC_JOB_H
 
 
+#include "signal.h"
 #include "signaller.h"
 #include <boost/atomic.hpp>
 #include <boost/signals2.hpp>
@@ -108,9 +109,9 @@ public:
 
 	void set_rate_limit_progress(bool rate_limit);
 
-	boost::signals2::signal<void()> Progress;
+	UISignal<void()> Progress;
 	/** Emitted from the UI thread when the job is finished */
-	boost::signals2::signal<void (Result)> Finished;
+	UISignal<void (Result)> Finished;
 	/** Emitted from the job thread when the job is finished */
 	boost::signals2::signal<void (Result)> FinishedImmediate;
 
diff --git a/src/lib/job_manager.cc b/src/lib/job_manager.cc
index c429dab6e..735b8e797 100644
--- a/src/lib/job_manager.cc
+++ b/src/lib/job_manager.cc
@@ -93,7 +93,7 @@ JobManager::add(shared_ptr<Job> j)
 		_schedule_condition.notify_all();
 	}
 
-	emit(boost::bind(boost::ref(JobAdded), weak_ptr<Job>(j)));
+	JobAdded.emit_ui(this, weak_ptr<Job>(j));
 
 	return j;
 }
@@ -110,7 +110,7 @@ JobManager::add_after(shared_ptr<Job> after, shared_ptr<Job> j)
 		_schedule_condition.notify_all();
 	}
 
-	emit(boost::bind(boost::ref(JobAdded), weak_ptr<Job>(j)));
+	JobAdded.emit_ui(this, weak_ptr<Job>(j));
 
 	return j;
 }
@@ -178,7 +178,7 @@ JobManager::scheduler()
 					i->resume();
 				}
 				auto last = _last_active_job.lock();
-				emit(boost::bind(boost::ref(ActiveJobsChanged), last ? last->json_name() : std::string{}, i->json_name()));
+				ActiveJobsChanged.emit_ui(this, last ? last->json_name() : std::string{}, i->json_name());
 				_last_active_job = i;
 				have_running = true;
 			} else if (!have_running && i->running()) {
@@ -197,7 +197,7 @@ JobManager::job_finished()
 	{
 		boost::mutex::scoped_lock lm(_mutex);
 		auto job = _last_active_job.lock();
-		emit(boost::bind(boost::ref(ActiveJobsChanged), job ? job->json_name() : string{}, optional<string>()));
+		ActiveJobsChanged.emit_ui(this, job ? job->json_name() : string{}, optional<string>());
 		_last_active_job = {};
 	}
 
@@ -257,7 +257,7 @@ JobManager::analyse_audio(
 		_schedule_condition.notify_all();
 	}
 
-	emit(boost::bind(boost::ref(JobAdded), weak_ptr<Job>(job)));
+	JobAdded.emit_ui(this, weak_ptr<Job>(job));
 }
 
 
@@ -292,7 +292,7 @@ JobManager::analyse_subtitles(
 		_schedule_condition.notify_all();
 	}
 
-	emit(boost::bind(boost::ref(JobAdded), weak_ptr<Job>(job)));
+	JobAdded.emit_ui(this, weak_ptr<Job>(job));
 }
 
 
@@ -309,7 +309,7 @@ JobManager::increase_priority (shared_ptr<Job> job)
 	}
 
 	_schedule_condition.notify_all();
-	emit(boost::bind(boost::ref(JobsReordered)));
+	JobsReordered.emit_ui(this);
 }
 
 
@@ -326,7 +326,7 @@ JobManager::decrease_priority(shared_ptr<Job> job)
 	}
 
 	_schedule_condition.notify_all();
-	emit(boost::bind(boost::ref(JobsReordered)));
+	JobsReordered.emit_ui(this);
 }
 
 
diff --git a/src/lib/job_manager.h b/src/lib/job_manager.h
index 248639782..f7981df41 100644
--- a/src/lib/job_manager.h
+++ b/src/lib/job_manager.h
@@ -25,6 +25,7 @@
 
 
 #include "job.h"
+#include "signal.h"
 #include "signaller.h"
 #include <boost/thread/mutex.hpp>
 #include <boost/thread.hpp>
@@ -86,9 +87,9 @@ public:
 
 	void cancel_all_jobs();
 
-	boost::signals2::signal<void (std::weak_ptr<Job>)> JobAdded;
-	boost::signals2::signal<void ()> JobsReordered;
-	boost::signals2::signal<void (boost::optional<std::string>, boost::optional<std::string>)> ActiveJobsChanged;
+	UISignal<void (std::weak_ptr<Job>)> JobAdded;
+	UISignal<void ()> JobsReordered;
+	UISignal<void (boost::optional<std::string>, boost::optional<std::string>)> ActiveJobsChanged;
 
 	static JobManager* instance();
 	static void drop();
diff --git a/src/lib/player.h b/src/lib/player.h
index 9076ac242..bd8fc1c52 100644
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -39,6 +39,7 @@
 #include "player_text.h"
 #include "position_image.h"
 #include "shuffler.h"
+#include "signal.h"
 #include <boost/atomic.hpp>
 #include <list>
 
@@ -119,7 +120,7 @@ public:
 	 *  Second parameter is the property.
 	 *  Third parameter is true if these signals are currently likely to be frequent.
 	 */
-	boost::signals2::signal<void (ChangeType, int, bool)> Change;
+	UISignal<void (ChangeType, int, bool)> Change;
 
 	/** Emitted when a video frame is ready.  These emissions happen in the correct order. */
 	boost::signals2::signal<void (std::shared_ptr<PlayerVideo>, dcpomatic::DCPTime)> Video;
diff --git a/src/lib/signal.h b/src/lib/signal.h
new file mode 100644
index 000000000..03b440e79
--- /dev/null
+++ b/src/lib/signal.h
@@ -0,0 +1,70 @@
+/*
+    Copyright (C) 2013-2021 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic 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.
+
+    DCP-o-matic 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 DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+
+#ifndef DCPOMATIC_SIGNAL_H
+#define DCPOMATIC_SIGNAL_H
+
+
+#include "signaller.h"
+#include "util.h"
+#include <boost/signals2.hpp>
+#include <utility>
+
+
+/** Wrapper for a boost::signals2::signal that asserts that emissions are made from the
+ *  UI thead, so anything connected to it will be guaranteed to be called from there.
+ */
+template <typename T>
+class UISignal
+{
+public:
+	template <typename... Args>
+	void emit_ui(Signaller* signaller, Args&&... args) const
+	{
+		signaller->emit(boost::bind(boost::ref(_wrapped), std::forward<Args>(args)...));
+	}
+
+	template <typename... Args>
+	void emit(Args&&... args) const
+	{
+		ensure_ui_thread();
+		_wrapped(std::forward<Args>(args)...);
+	}
+
+	template <typename... Args>
+	boost::signals2::connection connect(Args&&... args)
+	{
+		return _wrapped.connect(std::forward<Args>(args)...);
+	}
+
+	template <typename... Args>
+	void operator()(Args&&... args) const
+	{
+		emit(std::forward<Args>(args)...);
+	}
+
+private:
+	boost::signals2::signal<T> _wrapped;
+};
+
+
+#endif
+