Protect _threads with a mutex since it may be updated from

the GUI thread (via ServersChanged) or from the job thread
(via end ()).

2 files changed, 22 insertions, 11 deletions

diff --git a/src/lib/encoder.cc b/src/lib/encoder.cc
index b35f49a1b..00e28e1eb 100644
--- a/src/lib/encoder.cc
+++ b/src/lib/encoder.cc
@@ -83,7 +83,7 @@ Encoder::begin ()
 void
 Encoder::end ()
 {
-	boost::mutex::scoped_lock lock (_mutex);
+	boost::mutex::scoped_lock lock (_queue_mutex);
 
 	LOG_GENERAL (N_("Clearing queue of %1"), _queue.size ());
 
@@ -172,14 +172,20 @@ Encoder::enqueue (shared_ptr<PlayerVideo> pv)
 {
 	_waker.nudge ();
 
-	boost::mutex::scoped_lock lock (_mutex);
+	size_t threads = 0;
+	{
+		boost::mutex::scoped_lock threads_lock (_threads_mutex);
+		threads = _threads.size ();
+	}
+
+	boost::mutex::scoped_lock queue_lock (_queue_mutex);
 
 	/* XXX: discard 3D here if required */
 
 	/* Wait until the queue has gone down a bit */
-	while (_queue.size() >= _threads.size() * 2 && !_terminate) {
+	while (_queue.size() >= threads * 2 && !_terminate) {
 		LOG_TIMING ("decoder-sleep queue=%1", _queue.size());
-		_full_condition.wait (lock);
+		_full_condition.wait (queue_lock);
 		LOG_TIMING ("decoder-wake queue=%1", _queue.size());
 	}
 
@@ -249,12 +255,14 @@ void
 Encoder::terminate_threads ()
 {
 	{
-		boost::mutex::scoped_lock lock (_mutex);
+		boost::mutex::scoped_lock queue_lock (_queue_mutex);
 		_terminate = true;
 		_full_condition.notify_all ();
 		_empty_condition.notify_all ();
 	}
 
+	boost::mutex::scoped_lock threads_lock (_threads_mutex);
+
 	for (list<boost::thread *>::iterator i = _threads.begin(); i != _threads.end(); ++i) {
 		if ((*i)->joinable ()) {
 			(*i)->join ();
@@ -285,7 +293,7 @@ try
 	while (true) {
 
 		LOG_TIMING ("encoder-sleep thread=%1", boost::this_thread::get_id());
-		boost::mutex::scoped_lock lock (_mutex);
+		boost::mutex::scoped_lock lock (_queue_mutex);
 		while (_queue.empty () && !_terminate) {
 			_empty_condition.wait (lock);
 		}
@@ -367,6 +375,8 @@ Encoder::servers_list_changed ()
 
 	/* XXX: could re-use threads */
 
+	boost::mutex::scoped_lock lm (_threads_mutex);
+
 	if (!Config::instance()->only_servers_encode ()) {
 		for (int i = 0; i < Config::instance()->num_local_encoding_threads (); ++i) {
 			_threads.push_back (new boost::thread (boost::bind (&Encoder::encoder_thread, this, optional<ServerDescription> ())));
diff --git a/src/lib/encoder.h b/src/lib/encoder.h
index fbae07a9a..9b1b528bb 100644
--- a/src/lib/encoder.h
+++ b/src/lib/encoder.h
@@ -82,7 +82,7 @@ private:
 	boost::shared_ptr<const Film> _film;
 	boost::weak_ptr<Job> _job;
 
-	/** Mutex for _time_history and _last_frame */
+	/** Mutex for _time_history and _video_frames_enqueued */
 	mutable boost::mutex _state_mutex;
 	/** List of the times of completion of the last _history_size frames;
 	    first is the most recently completed.
@@ -90,16 +90,17 @@ private:
 	std::list<struct timeval> _time_history;
 	/** Number of frames that we should keep history for */
 	static int const _history_size;
-
 	/** Number of video frames enqueued so far */
 	int _video_frames_enqueued;
 	bool _left_done;
 	bool _right_done;
 
-	bool _terminate;
-	std::list<boost::shared_ptr<DCPVideo> > _queue;
+	boost::atomic<bool> _terminate;
+	/** Mutex for _threads */
+	mutable boost::mutex _threads_mutex;
 	std::list<boost::thread *> _threads;
-	mutable boost::mutex _mutex;
+	mutable boost::mutex _queue_mutex;
+	std::list<boost::shared_ptr<DCPVideo> > _queue;
 	/** condition to manage thread wakeups when we have nothing to do */
 	boost::condition _empty_condition;
 	/** condition to manage thread wakeups when we have too much to do */