From e1cab52c8437d8a985943faeb117da3b7da764b2 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 10 Feb 2018 19:38:26 -0500 Subject: [PATCH] Ensures thread safety of access to machines array. --- .../Implementation/MultiCRTMachine.cpp | 31 ++++++++++++------- .../Implementation/MultiCRTMachine.hpp | 4 ++- .../MultiConfigurationTarget.cpp | 15 ++++----- .../MultiConfigurationTarget.hpp | 4 +-- .../Dynamic/MultiMachine/MultiMachine.cpp | 4 ++- .../Dynamic/MultiMachine/MultiMachine.hpp | 2 ++ .../xcschemes/Clock Signal.xcscheme | 1 + 7 files changed, 39 insertions(+), 22 deletions(-) diff --git a/Analyser/Dynamic/MultiMachine/Implementation/MultiCRTMachine.cpp b/Analyser/Dynamic/MultiMachine/Implementation/MultiCRTMachine.cpp index a735b5a76..a18c39c13 100644 --- a/Analyser/Dynamic/MultiMachine/Implementation/MultiCRTMachine.cpp +++ b/Analyser/Dynamic/MultiMachine/Implementation/MultiCRTMachine.cpp @@ -13,25 +13,29 @@ using namespace Analyser::Dynamic; -MultiCRTMachine::MultiCRTMachine(const std::vector> &machines) : - machines_(machines), queues_(machines.size()) {} +MultiCRTMachine::MultiCRTMachine(const std::vector> &machines, std::mutex &machines_mutex) : + machines_(machines), machines_mutex_(machines_mutex), queues_(machines.size()) {} void MultiCRTMachine::perform_parallel(const std::function &function) { // Apply a blunt force parallelisation of the machines; each run_for is dispatched // to a separate queue and this queue will block until all are done. + volatile std::size_t outstanding_machines; std::condition_variable condition; std::mutex mutex; - std::size_t outstanding_machines = machines_.size(); + { + std::lock_guard machines_lock(machines_mutex_); + outstanding_machines = machines_.size(); - for(std::size_t index = 0; index < machines_.size(); ++index) { - queues_[index].enqueue([&mutex, &condition, this, index, function, &outstanding_machines]() { - CRTMachine::Machine *crt_machine = machines_[index]->crt_machine(); - if(crt_machine) function(crt_machine); + for(std::size_t index = 0; index < machines_.size(); ++index) { + queues_[index].enqueue([&mutex, &condition, this, index, function, &outstanding_machines]() { + CRTMachine::Machine *crt_machine = machines_[index]->crt_machine(); + if(crt_machine) function(crt_machine); - std::unique_lock lock(mutex); - outstanding_machines--; - condition.notify_all(); - }); + std::unique_lock lock(mutex); + outstanding_machines--; + condition.notify_all(); + }); + } } while(true) { @@ -42,6 +46,7 @@ void MultiCRTMachine::perform_parallel(const std::function &function) { + std::lock_guard machines_lock(machines_mutex_); for(const auto &machine: machines_) { CRTMachine::Machine *crt_machine = machine->crt_machine(); if(crt_machine) function(crt_machine); @@ -61,11 +66,13 @@ void MultiCRTMachine::close_output() { } Outputs::CRT::CRT *MultiCRTMachine::get_crt() { + std::lock_guard machines_lock(machines_mutex_); CRTMachine::Machine *crt_machine = machines_.front()->crt_machine(); return crt_machine ? crt_machine->get_crt() : nullptr; } Outputs::Speaker::Speaker *MultiCRTMachine::get_speaker() { + std::lock_guard machines_lock(machines_mutex_); CRTMachine::Machine *crt_machine = machines_.front()->crt_machine(); return crt_machine ? crt_machine->get_speaker() : nullptr; } @@ -80,11 +87,13 @@ void MultiCRTMachine::run_for(const Cycles cycles) { double MultiCRTMachine::get_clock_rate() { // TODO: something smarter than this? Not all clock rates will necessarily be the same. + std::lock_guard machines_lock(machines_mutex_); CRTMachine::Machine *crt_machine = machines_.front()->crt_machine(); return crt_machine ? crt_machine->get_clock_rate() : 0.0; } bool MultiCRTMachine::get_clock_is_unlimited() { + std::lock_guard machines_lock(machines_mutex_); CRTMachine::Machine *crt_machine = machines_.front()->crt_machine(); return crt_machine ? crt_machine->get_clock_is_unlimited() : false; } diff --git a/Analyser/Dynamic/MultiMachine/Implementation/MultiCRTMachine.hpp b/Analyser/Dynamic/MultiMachine/Implementation/MultiCRTMachine.hpp index 1152483d9..5a2fe112f 100644 --- a/Analyser/Dynamic/MultiMachine/Implementation/MultiCRTMachine.hpp +++ b/Analyser/Dynamic/MultiMachine/Implementation/MultiCRTMachine.hpp @@ -14,6 +14,7 @@ #include "../../../../Machines/DynamicMachine.hpp" #include +#include #include namespace Analyser { @@ -21,7 +22,7 @@ namespace Dynamic { class MultiCRTMachine: public ::CRTMachine::Machine, public ::CRTMachine::Machine::Delegate { public: - MultiCRTMachine(const std::vector> &machines); + MultiCRTMachine(const std::vector> &machines, std::mutex &machines_mutex); void setup_output(float aspect_ratio) override; void close_output() override; @@ -46,6 +47,7 @@ class MultiCRTMachine: public ::CRTMachine::Machine, public ::CRTMachine::Machin private: const std::vector> &machines_; + std::mutex &machines_mutex_; std::vector queues_; Delegate *delegate_ = nullptr; diff --git a/Analyser/Dynamic/MultiMachine/Implementation/MultiConfigurationTarget.cpp b/Analyser/Dynamic/MultiMachine/Implementation/MultiConfigurationTarget.cpp index da13ca227..22ae0e6ee 100644 --- a/Analyser/Dynamic/MultiMachine/Implementation/MultiConfigurationTarget.cpp +++ b/Analyser/Dynamic/MultiMachine/Implementation/MultiConfigurationTarget.cpp @@ -10,19 +10,20 @@ using namespace Analyser::Dynamic; -MultiConfigurationTarget::MultiConfigurationTarget(const std::vector> &machines) : - machines_(machines) {} +MultiConfigurationTarget::MultiConfigurationTarget(const std::vector> &machines) { + for(const auto &machine: machines) { + ConfigurationTarget::Machine *configuration_target = machine->configuration_target(); + if(configuration_target) targets_.push_back(configuration_target); + } +} void MultiConfigurationTarget::configure_as_target(const Analyser::Static::Target &target) { } bool MultiConfigurationTarget::insert_media(const Analyser::Static::Media &media) { bool inserted = false; - for(const auto &machine : machines_) { - ConfigurationTarget::Machine *configuration_target = machine->configuration_target(); - if(configuration_target) { - inserted |= configuration_target->insert_media(media); - } + for(const auto &target : targets_) { + inserted |= target->insert_media(media); } return inserted; } diff --git a/Analyser/Dynamic/MultiMachine/Implementation/MultiConfigurationTarget.hpp b/Analyser/Dynamic/MultiMachine/Implementation/MultiConfigurationTarget.hpp index f8790ce9d..886b6db40 100644 --- a/Analyser/Dynamic/MultiMachine/Implementation/MultiConfigurationTarget.hpp +++ b/Analyser/Dynamic/MultiMachine/Implementation/MultiConfigurationTarget.hpp @@ -18,7 +18,7 @@ namespace Analyser { namespace Dynamic { -struct MultiConfigurationTarget: public ::ConfigurationTarget::Machine { +struct MultiConfigurationTarget: public ConfigurationTarget::Machine { public: MultiConfigurationTarget(const std::vector> &machines); @@ -26,7 +26,7 @@ struct MultiConfigurationTarget: public ::ConfigurationTarget::Machine { bool insert_media(const Analyser::Static::Media &media) override; private: - const std::vector> &machines_; + std::vector targets_; }; } diff --git a/Analyser/Dynamic/MultiMachine/MultiMachine.cpp b/Analyser/Dynamic/MultiMachine/MultiMachine.cpp index daceb8a7a..ad721ded7 100644 --- a/Analyser/Dynamic/MultiMachine/MultiMachine.cpp +++ b/Analyser/Dynamic/MultiMachine/MultiMachine.cpp @@ -14,7 +14,7 @@ MultiMachine::MultiMachine(std::vector> &&machin machines_(std::move(machines)), configurable_(machines_), configuration_target_(machines_), - crt_machine_(machines_), + crt_machine_(machines_, machines_mutex_), joystick_machine_(machines), keyboard_machine_(machines_) { crt_machine_.set_delegate(this); @@ -41,6 +41,7 @@ Configurable::Device *MultiMachine::configurable_device() { } void MultiMachine::multi_crt_did_run_machines() { + std::lock_guard machines_lock(machines_mutex_); DynamicMachine *front = machines_.front().get(); // for(const auto &machine: machines_) { // CRTMachine::Machine *crt = machine->crt_machine(); @@ -49,6 +50,7 @@ void MultiMachine::multi_crt_did_run_machines() { // printf("; "); // } // printf("\n"); + std::stable_sort(machines_.begin(), machines_.end(), [] (const auto &lhs, const auto &rhs){ CRTMachine::Machine *lhs_crt = lhs->crt_machine(); CRTMachine::Machine *rhs_crt = rhs->crt_machine(); diff --git a/Analyser/Dynamic/MultiMachine/MultiMachine.hpp b/Analyser/Dynamic/MultiMachine/MultiMachine.hpp index 6afa5bd22..66b267eff 100644 --- a/Analyser/Dynamic/MultiMachine/MultiMachine.hpp +++ b/Analyser/Dynamic/MultiMachine/MultiMachine.hpp @@ -18,6 +18,7 @@ #include "Implementation/MultiKeyboardMachine.hpp" #include +#include #include namespace Analyser { @@ -49,6 +50,7 @@ class MultiMachine: public ::Machine::DynamicMachine, public MultiCRTMachine::De private: std::vector> machines_; + std::mutex machines_mutex_; MultiConfigurable configurable_; MultiConfigurationTarget configuration_target_; diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme index 46dd90eea..ebe496fa1 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -79,6 +79,7 @@ useCustomWorkingDirectory = "NO" ignoresPersistentStateOnLaunch = "NO" debugDocumentVersioning = "YES" + stopOnEveryThreadSanitizerIssue = "YES" stopOnEveryUBSanitizerIssue = "YES" debugServiceExtension = "internal" allowLocationSimulation = "NO">