It feels unsafe not to lock _threads_mutex between terminate_threads()

and _threads.reset(); move the lock.

1 files changed, 9 insertions, 4 deletions

diff --git a/src/lib/j2k_encoder.cc b/src/lib/j2k_encoder.cc
index c000f8599..9898ede78 100644
--- a/src/lib/j2k_encoder.cc
+++ b/src/lib/j2k_encoder.cc
@@ -64,6 +64,7 @@ J2KEncoder::J2KEncoder (shared_ptr<const Film> film, shared_ptr<Writer> writer)
 
 J2KEncoder::~J2KEncoder ()
 {
+	boost::mutex::scoped_lock lm (_threads_mutex);
 	terminate_threads ();
 }
 
@@ -107,7 +108,10 @@ J2KEncoder::end ()
 
 	LOG_GENERAL_NC (N_("Terminating encoder threads"));
 
-	terminate_threads ();
+	{
+		boost::mutex::scoped_lock lm (_threads_mutex);
+		terminate_threads ();
+	}
 
 	/* Something might have been thrown during terminate_threads */
 	rethrow ();
@@ -240,11 +244,12 @@ J2KEncoder::encode (shared_ptr<PlayerVideo> pv, DCPTime time)
 	_last_player_video_time = time;
 }
 
+
+/** Caller must hold a lock on _threads_mutex */
 void
 J2KEncoder::terminate_threads ()
 {
 	boost::this_thread::disable_interruption dis;
-	boost::mutex::scoped_lock lm (_threads_mutex);
 
 	if (!_threads) {
 		return;
@@ -372,11 +377,11 @@ catch (...)
 void
 J2KEncoder::servers_list_changed ()
 {
+	boost::mutex::scoped_lock lm (_threads_mutex);
+
 	terminate_threads ();
 	_threads.reset (new boost::thread_group());
 
-	boost::mutex::scoped_lock lm (_threads_mutex);
-
 	/* XXX: could re-use threads */
 
 	if (!Config::instance()->only_servers_encode ()) {