From a12a943c99ba4aba122f91c93b078d2e87146b32 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 12 Jun 2025 00:27:33 +0200 Subject: Use a new UISignal which checks thread safety slightly. This adds a wrapper around signals2::signal which checks that emission happens from the GUI thread, for signals whose handlers must be called in the UI thread. I'm not sure how helpful it really is but maybe it catches some bad situations. --- src/lib/config.h | 3 +- src/lib/encode_server_finder.cc | 4 +-- src/lib/encode_server_finder.h | 3 +- src/lib/film.cc | 6 ++-- src/lib/film.h | 11 ++++--- src/lib/hints.cc | 14 ++++----- src/lib/hints.h | 10 +++--- src/lib/job.cc | 6 ++-- src/lib/job.h | 5 +-- src/lib/job_manager.cc | 16 +++++----- src/lib/job_manager.h | 7 +++-- src/lib/player.h | 3 +- src/lib/signal.h | 70 +++++++++++++++++++++++++++++++++++++++++ 13 files changed, 117 insertions(+), 41 deletions(-) create mode 100644 src/lib/signal.h (limited to 'src/lib') diff --git a/src/lib/config.h b/src/lib/config.h index c90790ebc..0fbc3b7e9 100644 --- a/src/lib/config.h +++ b/src/lib/config.h @@ -31,6 +31,7 @@ #include "enum_indexed_vector.h" #include "export_config.h" #include "rough_duration.h" +#include "signal.h" #include "state.h" #include "video_encoding.h" #include @@ -1250,7 +1251,7 @@ public: void changed(Property p = OTHER); - boost::signals2::signal Changed; + UISignal Changed; /** Emitted if read() failed on an existing Config file. There is nothing a listener can do about it: this is just for information. */ diff --git a/src/lib/encode_server_finder.cc b/src/lib/encode_server_finder.cc index 1a0329dd7..1bee8eb52 100644 --- a/src/lib/encode_server_finder.cc +++ b/src/lib/encode_server_finder.cc @@ -169,7 +169,7 @@ try } if (removed) { - emit (boost::bind(boost::ref(ServersListChanged))); + ServersListChanged.emit_ui(this); } boost::mutex::scoped_lock lm (_search_condition_mutex); @@ -269,7 +269,7 @@ EncodeServerFinder::handle_accept (boost::system::error_code ec) } if (changed) { - emit (boost::bind(boost::ref (ServersListChanged))); + ServersListChanged.emit_ui(this); } start_accept (); diff --git a/src/lib/encode_server_finder.h b/src/lib/encode_server_finder.h index 722786b77..d5db0234d 100644 --- a/src/lib/encode_server_finder.h +++ b/src/lib/encode_server_finder.h @@ -27,6 +27,7 @@ #include "config.h" #include "encode_server_description.h" #include "exception_store.h" +#include "signal.h" #include "signaller.h" #include #include @@ -53,7 +54,7 @@ public: std::list servers () const; /** Emitted whenever the list of servers changes */ - boost::signals2::signal ServersListChanged; + UISignal ServersListChanged; private: EncodeServerFinder (); diff --git a/src/lib/film.cc b/src/lib/film.cc index 09378696e..d7cb05930 100644 --- a/src/lib/film.cc +++ b/src/lib/film.cc @@ -1310,7 +1310,7 @@ Film::signal_change(ChangeType type, FilmProperty p) if (type == ChangeType::DONE) { set_dirty(true); - emit(boost::bind(boost::ref(Change), type, p)); + Change.emit_ui(this, type, p); if (p == FilmProperty::VIDEO_FRAME_RATE || p == FilmProperty::SEQUENCE) { /* We want to call Playlist::maybe_sequence but this must happen after the @@ -1577,7 +1577,7 @@ Film::playlist_content_change(ChangeType type, weak_ptr c, int p, bool } if (type == ChangeType::DONE) { - emit(boost::bind(boost::ref(ContentChange), type, c, p, frequent)); + ContentChange.emit_ui(this, type, c, p, frequent); if (!frequent) { check_settings_consistency(); } @@ -2325,7 +2325,7 @@ Film::set_dirty(bool dirty) auto const changed = dirty != _dirty; _dirty = dirty; if (changed) { - emit(boost::bind(boost::ref(DirtyChange), _dirty)); + DirtyChange.emit_ui(this, _dirty); } } diff --git a/src/lib/film.h b/src/lib/film.h index c14b0f4fa..9f066e628 100644 --- a/src/lib/film.h +++ b/src/lib/film.h @@ -38,6 +38,7 @@ #include "named_channel.h" #include "remembered_asset.h" #include "resolution.h" +#include "signal.h" #include "signaller.h" #include "territory_type.h" #include "transcode_job.h" @@ -440,20 +441,20 @@ public: boost::filesystem::path info_file(dcpomatic::DCPTimePeriod p) const; /** Emitted when some property has of the Film is about to change or has changed */ - mutable boost::signals2::signal Change; + mutable UISignal Change; /** Emitted when some property of our content has changed */ - mutable boost::signals2::signal, int, bool)> ContentChange; + mutable UISignal, int, bool)> ContentChange; /** Emitted when the film's length might have changed; this is not like a normal property as its value is derived from the playlist, so it has its own signal. */ - mutable boost::signals2::signal LengthChange; + mutable UISignal LengthChange; - boost::signals2::signal DirtyChange; + UISignal DirtyChange; /** Emitted when we have something important to tell the user */ - boost::signals2::signal Message; + UISignal Message; /** Current version number of the state file */ static int const current_state_version; diff --git a/src/lib/hints.cc b/src/lib/hints.cc index 17aa16d47..5e9756b2b 100644 --- a/src/lib/hints.cc +++ b/src/lib/hints.cc @@ -411,11 +411,11 @@ Hints::scan_content(shared_ptr film) } if (check_loudness_done && have_text) { - emit(boost::bind(boost::ref(Progress), _("Examining subtitles and closed captions"))); + Progress.emit_ui(this, _("Examining subtitles and closed captions")); } else if (!check_loudness_done && !have_text) { - emit(boost::bind(boost::ref(Progress), _("Examining audio"))); + Progress.emit_ui(this, _("Examining audio")); } else { - emit(boost::bind(boost::ref(Progress), _("Examining audio, subtitles and closed captions"))); + Progress.emit_ui(this, _("Examining audio, subtitles and closed captions")); } auto player = make_shared(film, Image::Alignment::COMPACT, false); @@ -442,7 +442,7 @@ Hints::scan_content(shared_ptr film) if (_stop) { return; } - emit(boost::bind(boost::ref(Pulse))); + Pulse.emit_ui(this); last_pulse = now; } } @@ -536,7 +536,7 @@ try } dcp::filesystem::remove_all(dcp_dir); - emit(boost::bind(boost::ref(Finished))); + Finished.emit_ui(this); } catch (boost::thread_interrupted) { @@ -545,14 +545,14 @@ catch (boost::thread_interrupted) catch (...) { store_current(); - emit(boost::bind(boost::ref(Finished))); + Finished.emit_ui(this); } void Hints::hint(string h) { - emit(boost::bind(boost::ref(Hint), h)); + Hint.emit_ui(this, h); } diff --git a/src/lib/hints.h b/src/lib/hints.h index 49fd32ea9..56a5eb787 100644 --- a/src/lib/hints.h +++ b/src/lib/hints.h @@ -23,11 +23,11 @@ #include "dcp_text_track.h" #include "dcpomatic_time.h" #include "player_text.h" +#include "signal.h" #include "signaller.h" #include "text_type.h" #include "weak_film.h" #include -#include #include #include @@ -44,10 +44,10 @@ public: void start(); - boost::signals2::signal Hint; - boost::signals2::signal Progress; - boost::signals2::signal Pulse; - boost::signals2::signal Finished; + UISignal Hint; + UISignal Progress; + UISignal Pulse; + UISignal Finished; /* For tests only */ void join(); diff --git a/src/lib/job.cc b/src/lib/job.cc index ee6ad4e70..9f28c2bc2 100644 --- a/src/lib/job.cc +++ b/src/lib/job.cc @@ -375,7 +375,7 @@ Job::set_state (State s) if (finished) { auto const result = state_to_result(s); - emit(boost::bind(boost::ref(Finished), result)); + Finished.emit_ui(this, result); FinishedImmediate(result); } } @@ -419,7 +419,7 @@ Job::check_for_interruption_or_pause () boost::mutex::scoped_lock lm (_state_mutex); while (_state == PAUSED_BY_USER || _state == PAUSED_BY_PRIORITY) { - emit (boost::bind (boost::ref (Progress))); + Progress.emit_ui(this); _pause_changed.wait (lm); } } @@ -475,7 +475,7 @@ Job::set_progress_common (optional p) _progress = p; } - emit (boost::bind (boost::ref (Progress))); + Progress.emit_ui(this); } diff --git a/src/lib/job.h b/src/lib/job.h index 9b5fdfa6e..a80d66cda 100644 --- a/src/lib/job.h +++ b/src/lib/job.h @@ -28,6 +28,7 @@ #define DCPOMATIC_JOB_H +#include "signal.h" #include "signaller.h" #include #include @@ -108,9 +109,9 @@ public: void set_rate_limit_progress(bool rate_limit); - boost::signals2::signal Progress; + UISignal Progress; /** Emitted from the UI thread when the job is finished */ - boost::signals2::signal Finished; + UISignal Finished; /** Emitted from the job thread when the job is finished */ boost::signals2::signal FinishedImmediate; diff --git a/src/lib/job_manager.cc b/src/lib/job_manager.cc index c429dab6e..735b8e797 100644 --- a/src/lib/job_manager.cc +++ b/src/lib/job_manager.cc @@ -93,7 +93,7 @@ JobManager::add(shared_ptr j) _schedule_condition.notify_all(); } - emit(boost::bind(boost::ref(JobAdded), weak_ptr(j))); + JobAdded.emit_ui(this, weak_ptr(j)); return j; } @@ -110,7 +110,7 @@ JobManager::add_after(shared_ptr after, shared_ptr j) _schedule_condition.notify_all(); } - emit(boost::bind(boost::ref(JobAdded), weak_ptr(j))); + JobAdded.emit_ui(this, weak_ptr(j)); return j; } @@ -178,7 +178,7 @@ JobManager::scheduler() i->resume(); } auto last = _last_active_job.lock(); - emit(boost::bind(boost::ref(ActiveJobsChanged), last ? last->json_name() : std::string{}, i->json_name())); + ActiveJobsChanged.emit_ui(this, last ? last->json_name() : std::string{}, i->json_name()); _last_active_job = i; have_running = true; } else if (!have_running && i->running()) { @@ -197,7 +197,7 @@ JobManager::job_finished() { boost::mutex::scoped_lock lm(_mutex); auto job = _last_active_job.lock(); - emit(boost::bind(boost::ref(ActiveJobsChanged), job ? job->json_name() : string{}, optional())); + ActiveJobsChanged.emit_ui(this, job ? job->json_name() : string{}, optional()); _last_active_job = {}; } @@ -257,7 +257,7 @@ JobManager::analyse_audio( _schedule_condition.notify_all(); } - emit(boost::bind(boost::ref(JobAdded), weak_ptr(job))); + JobAdded.emit_ui(this, weak_ptr(job)); } @@ -292,7 +292,7 @@ JobManager::analyse_subtitles( _schedule_condition.notify_all(); } - emit(boost::bind(boost::ref(JobAdded), weak_ptr(job))); + JobAdded.emit_ui(this, weak_ptr(job)); } @@ -309,7 +309,7 @@ JobManager::increase_priority (shared_ptr job) } _schedule_condition.notify_all(); - emit(boost::bind(boost::ref(JobsReordered))); + JobsReordered.emit_ui(this); } @@ -326,7 +326,7 @@ JobManager::decrease_priority(shared_ptr job) } _schedule_condition.notify_all(); - emit(boost::bind(boost::ref(JobsReordered))); + JobsReordered.emit_ui(this); } diff --git a/src/lib/job_manager.h b/src/lib/job_manager.h index 248639782..f7981df41 100644 --- a/src/lib/job_manager.h +++ b/src/lib/job_manager.h @@ -25,6 +25,7 @@ #include "job.h" +#include "signal.h" #include "signaller.h" #include #include @@ -86,9 +87,9 @@ public: void cancel_all_jobs(); - boost::signals2::signal)> JobAdded; - boost::signals2::signal JobsReordered; - boost::signals2::signal, boost::optional)> ActiveJobsChanged; + UISignal)> JobAdded; + UISignal JobsReordered; + UISignal, boost::optional)> ActiveJobsChanged; static JobManager* instance(); static void drop(); diff --git a/src/lib/player.h b/src/lib/player.h index 9076ac242..bd8fc1c52 100644 --- a/src/lib/player.h +++ b/src/lib/player.h @@ -39,6 +39,7 @@ #include "player_text.h" #include "position_image.h" #include "shuffler.h" +#include "signal.h" #include #include @@ -119,7 +120,7 @@ public: * Second parameter is the property. * Third parameter is true if these signals are currently likely to be frequent. */ - boost::signals2::signal Change; + UISignal Change; /** Emitted when a video frame is ready. These emissions happen in the correct order. */ boost::signals2::signal, dcpomatic::DCPTime)> Video; diff --git a/src/lib/signal.h b/src/lib/signal.h new file mode 100644 index 000000000..03b440e79 --- /dev/null +++ b/src/lib/signal.h @@ -0,0 +1,70 @@ +/* + Copyright (C) 2013-2021 Carl Hetherington + + This file is part of DCP-o-matic. + + DCP-o-matic is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + DCP-o-matic is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with DCP-o-matic. If not, see . + +*/ + + +#ifndef DCPOMATIC_SIGNAL_H +#define DCPOMATIC_SIGNAL_H + + +#include "signaller.h" +#include "util.h" +#include +#include + + +/** Wrapper for a boost::signals2::signal that asserts that emissions are made from the + * UI thead, so anything connected to it will be guaranteed to be called from there. + */ +template +class UISignal +{ +public: + template + void emit_ui(Signaller* signaller, Args&&... args) const + { + signaller->emit(boost::bind(boost::ref(_wrapped), std::forward(args)...)); + } + + template + void emit(Args&&... args) const + { + ensure_ui_thread(); + _wrapped(std::forward(args)...); + } + + template + boost::signals2::connection connect(Args&&... args) + { + return _wrapped.connect(std::forward(args)...); + } + + template + void operator()(Args&&... args) const + { + emit(std::forward(args)...); + } + +private: + boost::signals2::signal _wrapped; +}; + + +#endif + -- cgit v1.2.3