From ea6b2dae46caa1da829fbf499e83cd6ae3b3773a Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 30 Jan 2020 22:54:38 +0100 Subject: [PATCH] Various thread cleanups. --- src/lib/butler.cc | 13 ++++++----- src/lib/butler.h | 2 +- src/lib/checker.cc | 21 ++++++------------ src/lib/checker.h | 2 +- src/lib/encode_server.cc | 33 ++++++++++++---------------- src/lib/encode_server.h | 7 +++--- src/lib/encode_server_finder.cc | 38 +++++++++++++-------------------- src/lib/encode_server_finder.h | 4 ++-- src/lib/j2k_encoder.cc | 21 ++++++------------ src/lib/j2k_encoder.h | 2 +- src/lib/job.cc | 34 +++++++++++------------------ src/lib/job.h | 2 +- src/lib/job_manager.cc | 18 ++++++---------- src/lib/job_manager.h | 2 +- src/lib/update_checker.cc | 17 ++++++--------- src/lib/update_checker.h | 2 +- src/lib/writer.cc | 16 +++++--------- src/lib/writer.h | 4 ++-- src/tools/dcpomatic_server.cc | 5 ++--- src/wx/gl_video_view.cc | 14 ++++++------ src/wx/gl_video_view.h | 2 +- 21 files changed, 105 insertions(+), 154 deletions(-) diff --git a/src/lib/butler.cc b/src/lib/butler.cc index 6062b0f21..20180330e 100644 --- a/src/lib/butler.cc +++ b/src/lib/butler.cc @@ -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 */ diff --git a/src/lib/butler.h b/src/lib/butler.h index ea4337443..0a0050e8f 100644 --- a/src/lib/butler.h +++ b/src/lib/butler.h @@ -73,7 +73,7 @@ private: void seek_unlocked (dcpomatic::DCPTime position, bool accurate); boost::shared_ptr _player; - boost::thread* _thread; + boost::thread _thread; VideoRingBuffers _video; AudioRingBuffers _audio; diff --git a/src/lib/checker.cc b/src/lib/checker.cc index 2d15dd3f1..c32462b42 100644 --- a/src/lib/checker.cc +++ b/src/lib/checker.cc @@ -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 diff --git a/src/lib/checker.h b/src/lib/checker.h index fee3fc3d9..8b80d8527 100644 --- a/src/lib/checker.h +++ b/src/lib/checker.h @@ -54,7 +54,7 @@ private: void thread (); - boost::thread* _thread; + boost::thread _thread; mutable boost::mutex _mutex; bool _terminate; bool _ok; diff --git a/src/lib/encode_server.cc b/src/lib/encode_server.cc index 314f8f68d..e79f82b62 100644 --- a/src/lib/encode_server.cc +++ b/src/lib/encode_server.cc @@ -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 (); diff --git a/src/lib/encode_server.h b/src/lib/encode_server.h index 14dc77398..40e84ad60 100644 --- a/src/lib/encode_server.h +++ b/src/lib/encode_server.h @@ -54,7 +54,7 @@ private: void broadcast_thread (); void broadcast_received (); - std::vector _worker_threads; + std::vector _worker_threads; std::list > _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; diff --git a/src/lib/encode_server_finder.cc b/src/lib/encode_server_finder.cc index 6cdd8ce3c..52c8c8949 100644 --- a/src/lib/encode_server_finder.cc +++ b/src/lib/encode_server_finder.cc @@ -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 (); diff --git a/src/lib/encode_server_finder.h b/src/lib/encode_server_finder.h index abfcc6d35..78b72fa9c 100644 --- a/src/lib/encode_server_finder.h +++ b/src/lib/encode_server_finder.h @@ -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 _servers; diff --git a/src/lib/j2k_encoder.cc b/src/lib/j2k_encoder.cc index 5c3fd477e..96b5b86e7 100644 --- a/src/lib/j2k_encoder.cc +++ b/src/lib/j2k_encoder.cc @@ -250,23 +250,17 @@ J2KEncoder::terminate_threads () boost::mutex::scoped_lock threads_lock (_threads_mutex); int n = 0; - for (list::iterator i = _threads.begin(); i != _threads.end(); ++i) { + for (list::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 ())); + _threads.push_back (boost::thread(boost::bind(&J2KEncoder::encoder_thread, this, optional()))); #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))); } } diff --git a/src/lib/j2k_encoder.h b/src/lib/j2k_encoder.h index b723ca88f..b57c4df7e 100644 --- a/src/lib/j2k_encoder.h +++ b/src/lib/j2k_encoder.h @@ -89,7 +89,7 @@ private: /** Mutex for _threads */ mutable boost::mutex _threads_mutex; - std::list _threads; + std::list _threads; mutable boost::mutex _queue_mutex; std::list > _queue; /** condition to manage thread wakeups when we have nothing to do */ diff --git a/src/lib/job.cc b/src/lib/job.cc index 8966a65e7..04aa227b7 100644 --- a/src/lib/job.cc +++ b/src/lib/job.cc @@ -51,7 +51,6 @@ using namespace dcpomatic; /** @param film Associated film, or 0 */ Job::Job (shared_ptr 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 */ diff --git a/src/lib/job.h b/src/lib/job.h index cb85059a6..a0e988fc8 100644 --- a/src/lib/job.h +++ b/src/lib/job.h @@ -121,7 +121,7 @@ private: void run_wrapper (); void set_progress_common (boost::optional p); - boost::thread* _thread; + boost::thread _thread; /** mutex for _state, _error*, _message */ mutable boost::mutex _state_mutex; diff --git a/src/lib/job_manager.cc b/src/lib/job_manager.cc index 26e2d2897..d95f95a24 100644 --- a/src/lib/job_manager.cc +++ b/src/lib/job_manager.cc @@ -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 diff --git a/src/lib/job_manager.h b/src/lib/job_manager.h index 2788fc657..d4287f874 100644 --- a/src/lib/job_manager.h +++ b/src/lib/job_manager.h @@ -93,7 +93,7 @@ private: boost::shared_ptr _paused_job; boost::optional _last_active_job; - boost::thread* _scheduler; + boost::thread _scheduler; static JobManager* _instance; }; diff --git a/src/lib/update_checker.cc b/src/lib/update_checker.cc index 98e407822..82337f920 100644 --- a/src/lib/update_checker.cc +++ b/src/lib/update_checker.cc @@ -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; diff --git a/src/lib/update_checker.h b/src/lib/update_checker.h index 5071bf4ec..78ca40303 100644 --- a/src/lib/update_checker.h +++ b/src/lib/update_checker.h @@ -93,7 +93,7 @@ private: boost::optional _test; int _emits; - boost::thread* _thread; + boost::thread _thread; boost::mutex _process_mutex; boost::condition _condition; int _to_do; diff --git a/src/lib/writer.cc b/src/lib/writer.cc index cc645c8b0..d85db5689 100644 --- a/src/lib/writer.cc +++ b/src/lib/writer.cc @@ -68,7 +68,6 @@ using namespace dcpomatic; Writer::Writer (shared_ptr film, weak_ptr 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 film, weak_ptr 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; } diff --git a/src/lib/writer.h b/src/lib/writer.h index 5fe96a3ac..d304133dc 100644 --- a/src/lib/writer.h +++ b/src/lib/writer.h @@ -131,8 +131,8 @@ private: std::vector::iterator _subtitle_reel; std::map::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 */ diff --git a/src/tools/dcpomatic_server.cc b/src/tools/dcpomatic_server.cc index 1554c02f4..8c6abed5b 100644 --- a/src/tools/dcpomatic_server.cc +++ b/src/tools/dcpomatic_server.cc @@ -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 _timer; }; diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index d47ad87f4..ebf4098e4 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -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)); } } diff --git a/src/wx/gl_video_view.h b/src/wx/gl_video_view.h index 2f3c8c2a1..84d97c751 100644 --- a/src/wx/gl_video_view.h +++ b/src/wx/gl_video_view.h @@ -68,7 +68,7 @@ private: boost::optional _size; bool _have_storage; bool _vsync_enabled; - boost::thread* _thread; + boost::thread _thread; boost::mutex _playing_mutex; boost::condition _playing_condition; -- 2.30.2