It feels unsafe not to lock _threads_mutex between terminate_threads()

and _threads.reset(); move the lock.

diff --git a/src/lib/j2k_encoder.cc b/src/lib/j2k_encoder.cc
index c000f8599cfa89a0b3aa70320ac6b805bcda9c43..9898ede787bb9b578892451fb00f28cb1f6205bb 100644 (file)
--- 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 ()) {