Various thread cleanups.
authorCarl Hetherington <cth@carlh.net>
Thu, 30 Jan 2020 21:54:38 +0000 (22:54 +0100)
committerCarl Hetherington <cth@carlh.net>
Thu, 30 Jan 2020 21:54:38 +0000 (22:54 +0100)
21 files changed:
src/lib/butler.cc
src/lib/butler.h
src/lib/checker.cc
src/lib/checker.h
src/lib/encode_server.cc
src/lib/encode_server.h
src/lib/encode_server_finder.cc
src/lib/encode_server_finder.h
src/lib/j2k_encoder.cc
src/lib/j2k_encoder.h
src/lib/job.cc
src/lib/job.h
src/lib/job_manager.cc
src/lib/job_manager.h
src/lib/update_checker.cc
src/lib/update_checker.h
src/lib/writer.cc
src/lib/writer.h
src/tools/dcpomatic_server.cc
src/wx/gl_video_view.cc
src/wx/gl_video_view.h

index 6062b0f..2018033 100644 (file)
@@ -83,9 +83,9 @@ Butler::Butler (
           get_video() to be called in response to this signal.
        */
        _player_change_connection = _player->Change.connect (bind (&Butler::player_change, this, _1), boost::signals2::at_front);
-       _thread = new boost::thread (bind (&Butler::thread, this));
+       _thread = boost::thread (bind(&Butler::thread, this));
 #ifdef DCPOMATIC_LINUX
-       pthread_setname_np (_thread->native_handle(), "butler");
+       pthread_setname_np (_thread.native_handle(), "butler");
 #endif
 
        /* Create some threads to do work on the PlayerVideos we are creating; at present this is used to
@@ -110,13 +110,12 @@ Butler::~Butler ()
        _prepare_pool.join_all ();
        _prepare_service.stop ();
 
-       _thread->interrupt ();
+       _thread.interrupt ();
        try {
-               _thread->join ();
-       } catch (boost::thread_interrupted& e) {
-               /* No problem */
+               _thread.join ();
+       } catch (...) {
+
        }
-       delete _thread;
 }
 
 /** Caller must hold a lock on _mutex */
index ea43374..0a0050e 100644 (file)
@@ -73,7 +73,7 @@ private:
        void seek_unlocked (dcpomatic::DCPTime position, bool accurate);
 
        boost::shared_ptr<Player> _player;
-       boost::thread* _thread;
+       boost::thread _thread;
 
        VideoRingBuffers _video;
        AudioRingBuffers _audio;
index 2d15dd3..c32462b 100644 (file)
@@ -28,8 +28,7 @@ using boost::bind;
 using boost::ref;
 
 Checker::Checker (int period)
-       : _thread (0)
-       , _terminate (false)
+       : _terminate (false)
        , _ok (true)
        , _period (period)
 {
@@ -39,7 +38,7 @@ Checker::Checker (int period)
 void
 Checker::run ()
 {
-       _thread = new boost::thread (boost::bind (&Checker::thread, this));
+       _thread = boost::thread (boost::bind(&Checker::thread, this));
 }
 
 Checker::~Checker ()
@@ -49,20 +48,14 @@ Checker::~Checker ()
                _terminate = true;
        }
 
-       if (_thread) {
-               /* Ideally this would be a DCPOMATIC_ASSERT(_thread->joinable()) but we
-                  can't throw exceptions from a destructor.
-               */
-               _thread->interrupt ();
+       if (_thread.joinable()) {
+               _thread.interrupt ();
                try {
-                       if (_thread->joinable ()) {
-                               _thread->join ();
-                       }
-               } catch (boost::thread_interrupted& e) {
-                       /* No problem */
+                       _thread.join ();
+               } catch (...) {
+
                }
        }
-       delete _thread;
 }
 
 void
index fee3fc3..8b80d85 100644 (file)
@@ -54,7 +54,7 @@ private:
 
        void thread ();
 
-       boost::thread* _thread;
+       boost::thread _thread;
        mutable boost::mutex _mutex;
        bool _terminate;
        bool _ok;
index 314f8f6..e79f82b 100644 (file)
@@ -87,14 +87,12 @@ EncodeServer::~EncodeServer ()
                _full_condition.notify_all ();
        }
 
-       BOOST_FOREACH (boost::thread* i, _worker_threads) {
-               /* Ideally this would be a DCPOMATIC_ASSERT(i->joinable()) but we
-                  can't throw exceptions from a destructor.
-               */
-               if (i->joinable ()) {
-                       i->join ();
+       BOOST_FOREACH (boost::thread& i, _worker_threads) {
+               try {
+                       i.join ();
+               } catch (...) {
+
                }
-               delete i;
        }
 
        {
@@ -107,14 +105,12 @@ EncodeServer::~EncodeServer ()
        }
 
        _broadcast.io_service.stop ();
-       if (_broadcast.thread) {
-               /* Ideally this would be a DCPOMATIC_ASSERT(_broadcast.thread->joinable()) but we
-                  can't throw exceptions from a destructor.
-               */
-               if (_broadcast.thread->joinable ()) {
-                       _broadcast.thread->join ();
+       if (_broadcast.thread.joinable()) {
+               try {
+                       _broadcast.thread.join ();
+               } catch (...) {
+
                }
-               delete _broadcast.thread;
        }
 }
 
@@ -237,16 +233,15 @@ EncodeServer::run ()
        }
 
        for (int i = 0; i < _num_threads; ++i) {
-               thread* t = new thread (bind (&EncodeServer::worker_thread, this));
+               _worker_threads.push_back (thread(bind(&EncodeServer::worker_thread, this)));
 #ifdef DCPOMATIC_LINUX
-               pthread_setname_np (t->native_handle(), "encode-server-worker");
+               pthread_setname_np (_worker_threads.back().native_handle(), "encode-server-worker");
 #endif
-               _worker_threads.push_back (t);
        }
 
-       _broadcast.thread = new thread (bind (&EncodeServer::broadcast_thread, this));
+       _broadcast.thread = thread (bind(&EncodeServer::broadcast_thread, this));
 #ifdef DCPOMATIC_LINUX
-       pthread_setname_np (_broadcast.thread->native_handle(), "encode-server-broadcast");
+       pthread_setname_np (_broadcast.thread.native_handle(), "encode-server-broadcast");
 #endif
 
        Server::run ();
index 14dc773..40e84ad 100644 (file)
@@ -54,7 +54,7 @@ private:
        void broadcast_thread ();
        void broadcast_received ();
 
-       std::vector<boost::thread *> _worker_threads;
+       std::vector<boost::thread> _worker_threads;
        std::list<boost::shared_ptr<Socket> > _queue;
        boost::condition _full_condition;
        boost::condition _empty_condition;
@@ -64,12 +64,11 @@ private:
        struct Broadcast {
 
                Broadcast ()
-                       : thread (0)
-                       , socket (0)
+                       : socket (0)
                {}
 
                boost::mutex mutex;
-               boost::thread* thread;
+               boost::thread thread;
                boost::asio::ip::udp::socket* socket;
                char buffer[64];
                boost::asio::ip::udp::endpoint send_endpoint;
index 6cdd8ce..52c8c89 100644 (file)
@@ -45,9 +45,7 @@ using dcp::raw_convert;
 EncodeServerFinder* EncodeServerFinder::_instance = 0;
 
 EncodeServerFinder::EncodeServerFinder ()
-       : _search_thread (0)
-       , _listen_thread (0)
-       , _stop (false)
+       : _stop (false)
 {
        Config::instance()->Changed.connect (boost::bind (&EncodeServerFinder::config_changed, this, _1));
 }
@@ -55,11 +53,11 @@ EncodeServerFinder::EncodeServerFinder ()
 void
 EncodeServerFinder::start ()
 {
-       _search_thread = new boost::thread (boost::bind (&EncodeServerFinder::search_thread, this));
-       _listen_thread = new boost::thread (boost::bind (&EncodeServerFinder::listen_thread, this));
+       _search_thread = boost::thread (boost::bind(&EncodeServerFinder::search_thread, this));
+       _listen_thread = boost::thread (boost::bind(&EncodeServerFinder::listen_thread, this));
 #ifdef DCPOMATIC_LINUX
-       pthread_setname_np (_search_thread->native_handle(), "encode-server-search");
-       pthread_setname_np (_listen_thread->native_handle(), "encode-server-listen");
+       pthread_setname_np (_search_thread.native_handle(), "encode-server-search");
+       pthread_setname_np (_listen_thread.native_handle(), "encode-server-listen");
 #endif
 }
 
@@ -75,28 +73,22 @@ EncodeServerFinder::stop ()
        _stop = true;
 
        _search_condition.notify_all ();
-       if (_search_thread) {
-               /* Ideally this would be a DCPOMATIC_ASSERT(_search_thread->joinable()) but we
-                  can't throw exceptions from a destructor.
-               */
-               if (_search_thread->joinable ()) {
-                       _search_thread->join ();
+       if (_search_thread.joinable()) {
+               try {
+                       _search_thread.join();
+               } catch (...) {
+
                }
        }
-       delete _search_thread;
-       _search_thread = 0;
 
        _listen_io_service.stop ();
-       if (_listen_thread) {
-               /* Ideally this would be a DCPOMATIC_ASSERT(_listen_thread->joinable()) but we
-                  can't throw exceptions from a destructor.
-               */
-               if (_listen_thread->joinable ()) {
-                       _listen_thread->join ();
+       if (_listen_thread.joinable()) {
+               try {
+                       _listen_thread.join ();
+               } catch (...) {
+
                }
        }
-       delete _listen_thread;
-       _listen_thread = 0;
 
        boost::mutex::scoped_lock lm (_servers_mutex);
        _servers.clear ();
index abfcc6d..78b72fa 100644 (file)
@@ -69,9 +69,9 @@ private:
        void config_changed (Config::Property what);
 
        /** Thread to periodically issue broadcasts and requests to find encoding servers */
-       boost::thread* _search_thread;
+       boost::thread _search_thread;
        /** Thread to listen to the responses from servers */
-       boost::thread* _listen_thread;
+       boost::thread _listen_thread;
 
        /** Available servers */
        std::list<EncodeServerDescription> _servers;
index 5c3fd47..96b5b86 100644 (file)
@@ -250,23 +250,17 @@ J2KEncoder::terminate_threads ()
        boost::mutex::scoped_lock threads_lock (_threads_mutex);
 
        int n = 0;
-       for (list<boost::thread *>::iterator i = _threads.begin(); i != _threads.end(); ++i) {
+       for (list<boost::thread>::iterator i = _threads.begin(); i != _threads.end(); ++i) {
                /* Be careful not to throw in here otherwise _threads will not be clear()ed */
                LOG_GENERAL ("Terminating thread %1 of %2", n + 1, _threads.size ());
-               (*i)->interrupt ();
-               if (!(*i)->joinable()) {
-                       LOG_ERROR_NC ("About to join() a non-joinable thread");
-               }
+               i->interrupt ();
                try {
-                       (*i)->join ();
-               } catch (boost::thread_interrupted& e) {
-                       /* This is to be expected (I think?) */
+                       i->join ();
                } catch (exception& e) {
                        LOG_ERROR ("join() threw an exception: %1", e.what());
                } catch (...) {
                        LOG_ERROR_NC ("join() threw an exception");
                }
-               delete *i;
                LOG_GENERAL_NC ("Thread terminated");
                ++n;
        }
@@ -402,14 +396,13 @@ J2KEncoder::servers_list_changed ()
 
        if (!Config::instance()->only_servers_encode ()) {
                for (int i = 0; i < Config::instance()->master_encoding_threads (); ++i) {
-                       boost::thread* t = new boost::thread (boost::bind (&J2KEncoder::encoder_thread, this, optional<EncodeServerDescription> ()));
+                       _threads.push_back (boost::thread(boost::bind(&J2KEncoder::encoder_thread, this, optional<EncodeServerDescription>())));
 #ifdef DCPOMATIC_LINUX
-                       pthread_setname_np (t->native_handle(), "encode-worker");
+                       pthread_setname_np (_threads.back().native_handle(), "encode-worker");
 #endif
-                       _threads.push_back (t);
 #ifdef BOOST_THREAD_PLATFORM_WIN32
                        if (windows_xp) {
-                               SetThreadAffinityMask (t->native_handle(), 1 << i);
+                               SetThreadAffinityMask (_threads.back().native_handle(), 1 << i);
                        }
 #endif
                }
@@ -422,7 +415,7 @@ J2KEncoder::servers_list_changed ()
 
                LOG_GENERAL (N_("Adding %1 worker threads for remote %2"), i.threads(), i.host_name ());
                for (int j = 0; j < i.threads(); ++j) {
-                       _threads.push_back (new boost::thread (boost::bind (&J2KEncoder::encoder_thread, this, i)));
+                       _threads.push_back (boost::thread(boost::bind(&J2KEncoder::encoder_thread, this, i)));
                }
        }
 
index b723ca8..b57c4df 100644 (file)
@@ -89,7 +89,7 @@ private:
 
        /** Mutex for _threads */
        mutable boost::mutex _threads_mutex;
-       std::list<boost::thread *> _threads;
+       std::list<boost::thread> _threads;
        mutable boost::mutex _queue_mutex;
        std::list<boost::shared_ptr<DCPVideo> > _queue;
        /** condition to manage thread wakeups when we have nothing to do */
index 8966a65..04aa227 100644 (file)
@@ -51,7 +51,6 @@ using namespace dcpomatic;
 /** @param film Associated film, or 0 */
 Job::Job (shared_ptr<const Film> film)
        : _film (film)
-       , _thread (0)
        , _state (NEW)
        , _start_time (0)
        , _sub_start_time (0)
@@ -69,20 +68,16 @@ Job::~Job ()
 void
 Job::stop_thread ()
 {
-       if (_thread) {
-               _thread->interrupt ();
-               /* We can't use DCPOMATIC_ASSERT here as it may throw an exception */
-               if (_thread->joinable ()) {
-                       try {
-                               _thread->join ();
-                       } catch (...) {
-                               /* Too late to do anything about this */
-                       }
-               }
+       if (!_thread.joinable()) {
+               return;
        }
 
-       delete _thread;
-       _thread = 0;
+       _thread.interrupt ();
+       try {
+               _thread.join ();
+       } catch (...) {
+               /* Too late to do anything about this */
+       }
 }
 
 /** Start the job in a separate thread, returning immediately */
@@ -92,9 +87,9 @@ Job::start ()
        set_state (RUNNING);
        _start_time = time (0);
        _sub_start_time = time (0);
-       _thread = new boost::thread (boost::bind (&Job::run_wrapper, this));
+       _thread = boost::thread (boost::bind(&Job::run_wrapper, this));
 #ifdef DCPOMATIC_LINUX
-       pthread_setname_np (_thread->native_handle(), "job-wrapper");
+       pthread_setname_np (_thread.native_handle(), "job-wrapper");
 #endif
 }
 
@@ -515,7 +510,7 @@ Job::remaining_time () const
 void
 Job::cancel ()
 {
-       if (!_thread) {
+       if (!_thread.joinable()) {
                return;
        }
 
@@ -523,11 +518,8 @@ Job::cancel ()
                resume ();
        }
 
-       _thread->interrupt ();
-       DCPOMATIC_ASSERT (_thread->joinable ());
-       _thread->join ();
-       delete _thread;
-       _thread = 0;
+       _thread.interrupt ();
+       _thread.join ();
 }
 
 /** @return true if the job was paused, false if it was not running */
index cb85059..a0e988f 100644 (file)
@@ -121,7 +121,7 @@ private:
        void run_wrapper ();
        void set_progress_common (boost::optional<float> p);
 
-       boost::thread* _thread;
+       boost::thread _thread;
 
        /** mutex for _state, _error*, _message */
        mutable boost::mutex _state_mutex;
index 26e2d28..d95f95a 100644 (file)
@@ -46,7 +46,6 @@ JobManager* JobManager::_instance = 0;
 JobManager::JobManager ()
        : _terminate (false)
        , _paused (false)
-       , _scheduler (0)
 {
 
 }
@@ -54,9 +53,9 @@ JobManager::JobManager ()
 void
 JobManager::start ()
 {
-       _scheduler = new boost::thread (boost::bind (&JobManager::scheduler, this));
+       _scheduler = boost::thread (boost::bind(&JobManager::scheduler, this));
 #ifdef DCPOMATIC_LINUX
-       pthread_setname_np (_scheduler->native_handle(), "job-scheduler");
+       pthread_setname_np (_scheduler.native_handle(), "job-scheduler");
 #endif
 }
 
@@ -72,16 +71,13 @@ JobManager::~JobManager ()
                _empty_condition.notify_all ();
        }
 
-       if (_scheduler) {
-               /* Ideally this would be a DCPOMATIC_ASSERT(_scheduler->joinable()) but we
-                  can't throw exceptions from a destructor.
-               */
-               if (_scheduler->joinable ()) {
-                       _scheduler->join ();
+       if (_scheduler.joinable()) {
+               try {
+                       _scheduler.join();
+               } catch (...) {
+
                }
        }
-
-       delete _scheduler;
 }
 
 shared_ptr<Job>
index 2788fc6..d4287f8 100644 (file)
@@ -93,7 +93,7 @@ private:
        boost::shared_ptr<Job> _paused_job;
 
        boost::optional<std::string> _last_active_job;
-       boost::thread* _scheduler;
+       boost::thread _scheduler;
 
        static JobManager* _instance;
 };
index 98e4078..82337f9 100644 (file)
@@ -56,7 +56,6 @@ UpdateChecker::UpdateChecker ()
        , _curl (0)
        , _state (NOT_RUN)
        , _emits (0)
-       , _thread (0)
        , _to_do (0)
        , _terminate (false)
 {
@@ -74,9 +73,9 @@ UpdateChecker::UpdateChecker ()
 void
 UpdateChecker::start ()
 {
-       _thread = new boost::thread (boost::bind (&UpdateChecker::thread, this));
+       _thread = boost::thread (boost::bind (&UpdateChecker::thread, this));
 #ifdef DCPOMATIC_LINUX
-       pthread_setname_np (_thread->native_handle(), "update-checker");
+       pthread_setname_np (_thread.native_handle(), "update-checker");
 #endif
 }
 
@@ -88,15 +87,13 @@ UpdateChecker::~UpdateChecker ()
        }
 
        _condition.notify_all ();
-       if (_thread) {
-               /* Ideally this would be a DCPOMATIC_ASSERT(_thread->joinable()) but we
-                  can't throw exceptions from a destructor.
-               */
-               if (_thread->joinable ()) {
-                       _thread->join ();
+       if (_thread.joinable()) {
+               try {
+                       _thread.join ();
+               } catch (...) {
+
                }
        }
-       delete _thread;
 
        curl_easy_cleanup (_curl);
        delete[] _buffer;
index 5071bf4..78ca403 100644 (file)
@@ -93,7 +93,7 @@ private:
        boost::optional<std::string> _test;
        int _emits;
 
-       boost::thread* _thread;
+       boost::thread _thread;
        boost::mutex _process_mutex;
        boost::condition _condition;
        int _to_do;
index cc645c8..d85db56 100644 (file)
@@ -68,7 +68,6 @@ using namespace dcpomatic;
 Writer::Writer (shared_ptr<const Film> film, weak_ptr<Job> j)
        : _film (film)
        , _job (j)
-       , _thread (0)
        , _finish (false)
        , _queued_full_in_memory (0)
        /* These will be reset to sensible values when J2KEncoder is created */
@@ -107,9 +106,9 @@ Writer::Writer (shared_ptr<const Film> film, weak_ptr<Job> j)
 void
 Writer::start ()
 {
-       _thread = new boost::thread (boost::bind (&Writer::thread, this));
+       _thread = boost::thread (boost::bind(&Writer::thread, this));
 #ifdef DCPOMATIC_LINUX
-       pthread_setname_np (_thread->native_handle(), "writer");
+       pthread_setname_np (_thread.native_handle(), "writer");
 #endif
 }
 
@@ -465,7 +464,7 @@ void
 Writer::terminate_thread (bool can_throw)
 {
        boost::mutex::scoped_lock lock (_state_mutex);
-       if (_thread == 0) {
+       if (!_thread.joinable()) {
                return;
        }
 
@@ -474,22 +473,17 @@ Writer::terminate_thread (bool can_throw)
        _full_condition.notify_all ();
        lock.unlock ();
 
-       if (_thread->joinable ()) {
-               _thread->join ();
-       }
+       _thread.join ();
 
        if (can_throw) {
                rethrow ();
        }
-
-       delete _thread;
-       _thread = 0;
 }
 
 void
 Writer::finish ()
 {
-       if (!_thread) {
+       if (!_thread.joinable()) {
                return;
        }
 
index 5fe96a3..d304133 100644 (file)
@@ -131,8 +131,8 @@ private:
        std::vector<ReelWriter>::iterator _subtitle_reel;
        std::map<DCPTextTrack, std::vector<ReelWriter>::iterator> _caption_reels;
 
-       /** our thread, or 0 */
-       boost::thread* _thread;
+       /** our thread */
+       boost::thread _thread;
        /** true if our thread should finish */
        bool _finish;
        /** queue of things to write to disk */
index 1554c02..8c6abed 100644 (file)
@@ -258,7 +258,6 @@ class App : public wxApp, public ExceptionStore
 public:
        App ()
                : wxApp ()
-               , _thread (0)
                , _icon (0)
        {}
 
@@ -302,7 +301,7 @@ private:
 #else
                _icon = new TaskBarIcon;
 #endif
-               _thread = new thread (bind (&App::main_thread, this));
+               _thread = thread (bind (&App::main_thread, this));
 
                Bind (wxEVT_TIMER, boost::bind (&App::check, this));
                _timer.reset (new wxTimer (this));
@@ -359,7 +358,7 @@ private:
                message_dialog (0, std_to_wx (m));
        }
 
-       boost::thread* _thread;
+       boost::thread _thread;
        TaskBarIcon* _icon;
        shared_ptr<wxTimer> _timer;
 };
index d47ad87..ebf4098 100644 (file)
@@ -57,7 +57,6 @@ GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent)
        : VideoView (viewer)
        , _have_storage (false)
        , _vsync_enabled (false)
-       , _thread (0)
        , _playing (false)
        , _one_shot (false)
 {
@@ -105,9 +104,12 @@ GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent)
 
 GLVideoView::~GLVideoView ()
 {
-       _thread->interrupt ();
-       _thread->join ();
-       delete _thread;
+       try {
+               _thread.interrupt ();
+               _thread.join ();
+       } catch (...) {
+
+       }
 
        glDeleteTextures (1, &_id);
 }
@@ -386,7 +388,7 @@ GLVideoView::request_one_shot ()
 void
 GLVideoView::create ()
 {
-       if (!_thread) {
-               _thread = new boost::thread (boost::bind(&GLVideoView::thread, this));
+       if (!_thread.joinable()) {
+               _thread = boost::thread (boost::bind(&GLVideoView::thread, this));
        }
 }
index 2f3c8c2..84d97c7 100644 (file)
@@ -68,7 +68,7 @@ private:
        boost::optional<dcp::Size> _size;
        bool _have_storage;
        bool _vsync_enabled;
-       boost::thread* _thread;
+       boost::thread _thread;
 
        boost::mutex _playing_mutex;
        boost::condition _playing_condition;