From 5cfe443078d561e16c3e1111ae50bb27e156d9b7 Mon Sep 17 00:00:00 2001 From: Alexandr Yanenko Date: Fri, 11 Jan 2019 12:12:57 +0300 Subject: [PATCH 1/2] Recording thread deadlock fix --- AirLib/include/common/WorkerThread.hpp | 28 +++++++ .../Source/Recording/RecordingThread.cpp | 76 +++++++++++++------ .../AirSim/Source/Recording/RecordingThread.h | 14 +++- .../AirSim/Source/SimMode/SimModeBase.cpp | 2 + 4 files changed, 91 insertions(+), 29 deletions(-) diff --git a/AirLib/include/common/WorkerThread.hpp b/AirLib/include/common/WorkerThread.hpp index 696583c55f..8ba1c501f0 100644 --- a/AirLib/include/common/WorkerThread.hpp +++ b/AirLib/include/common/WorkerThread.hpp @@ -102,6 +102,34 @@ class WorkerThreadSignal return true; } + void wait() + { + // wait for signal or timeout or cancel predicate + std::unique_lock lock(mutex_); + while (!signaled_) { + cv_.wait(lock); + } + lock.unlock(); + signaled_ = false; + } + + bool waitForRetry(double timeout_sec, int n_times) + { + std::unique_lock lock(mutex_); + while (!signaled_ && n_times > 0) { + cv_.wait_for(lock, std::chrono::milliseconds(static_cast(timeout_sec * 1000))); + --n_times; + } + lock.unlock(); + if (n_times == 0 && !signaled_) { + return false; + } + else { + signaled_ = false; + return true; + } + } + }; // This class provides a synchronized worker thread that guarantees to execute diff --git a/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.cpp b/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.cpp index 7338ccf7c6..9ec902c5d5 100644 --- a/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.cpp +++ b/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.cpp @@ -8,7 +8,10 @@ #include "PIPCamera.h" -std::unique_ptr FRecordingThread::instance_; +std::unique_ptr FRecordingThread::running_instance_; +std::unique_ptr FRecordingThread::finishing_instance_; +msr::airlib::WorkerThreadSignal FRecordingThread::finishing_signal_; +bool FRecordingThread::first_ = true; FRecordingThread::FRecordingThread() @@ -18,44 +21,62 @@ FRecordingThread::FRecordingThread() } -void FRecordingThread::startRecording(const msr::airlib::ImageCaptureBase* image_capture, const msr::airlib::Kinematics::State* kinematics, +void FRecordingThread::startRecording(const msr::airlib::ImageCaptureBase* image_capture, const msr::airlib::Kinematics::State* kinematics, const RecordingSetting& settings, msr::airlib::VehicleSimApiBase* vehicle_sim_api) { stopRecording(); //TODO: check FPlatformProcess::SupportsMultithreading()? + assert(!isRecording()); - instance_.reset(new FRecordingThread()); - instance_->image_capture_ = image_capture; - instance_->kinematics_ = kinematics; - instance_->settings_ = settings; - instance_->vehicle_sim_api_ = vehicle_sim_api; + running_instance_.reset(new FRecordingThread()); + running_instance_->image_capture_ = image_capture; + running_instance_->kinematics_ = kinematics; + running_instance_->settings_ = settings; + running_instance_->vehicle_sim_api_ = vehicle_sim_api; - instance_->last_screenshot_on_ = 0; - instance_->last_pose_ = msr::airlib::Pose(); + running_instance_->last_screenshot_on_ = 0; + running_instance_->last_pose_ = msr::airlib::Pose(); - instance_->is_ready_ = true; + running_instance_->is_ready_ = true; - instance_->recording_file_.reset(new RecordingFile()); - instance_->recording_file_->startRecording(vehicle_sim_api); + running_instance_->recording_file_.reset(new RecordingFile()); + running_instance_->recording_file_->startRecording(vehicle_sim_api); } FRecordingThread::~FRecordingThread() { - stopRecording(); + if (this == running_instance_.get()) stopRecording(); +} + +void FRecordingThread::init() +{ + first_ = true; } bool FRecordingThread::isRecording() { - return instance_ != nullptr; + return running_instance_ != nullptr; } void FRecordingThread::stopRecording() { - if (instance_) + if (running_instance_) { - instance_->EnsureCompletion(); - instance_.reset(); + assert(finishing_instance_ == nullptr); + finishing_instance_ = std::move(running_instance_); + assert(!isRecording()); + finishing_instance_->Stop(); + } +} + +void FRecordingThread::killRecording() +{ + stopRecording(); + bool finished = finishing_signal_.waitForRetry(1, 5); + if (!finished) { + UE_LOG(LogTemp, Log, TEXT("killing thread")); + finishing_instance_->thread_->Kill(false); } } @@ -63,6 +84,12 @@ void FRecordingThread::stopRecording() bool FRecordingThread::Init() { + if (first_) { + first_ = false; + } + else { + finishing_signal_.wait(); + } if (image_capture_ && recording_file_) { UAirBlueprintLib::LogMessage(TEXT("Initiated recording thread"), TEXT(""), LogDebugLevel::Success); @@ -85,8 +112,12 @@ uint32 FRecordingThread::Run() //TODO: should we go as fast as possible, or should we limit this to a particular number of // frames per second? - + + //BG: Workaround to get sync ground truth. See https://github.com/Microsoft/AirSim/issues/1494 for details + uint64_t timestamp_millis = static_cast(msr::airlib::ClockFactory::get()->nowNanos() / 1.0E6); + std::string gt = vehicle_sim_api_->getRecordFileLine(false); std::vector responses; + image_capture_->getImages(settings_.requests, responses); recording_file_->appendRecord(responses, vehicle_sim_api_); } @@ -107,13 +138,8 @@ void FRecordingThread::Stop() void FRecordingThread::Exit() { + assert(this == finishing_instance_.get()); if (recording_file_) recording_file_.reset(); -} - -void FRecordingThread::EnsureCompletion() -{ - Stop(); - thread_->WaitForCompletion(); - //UAirBlueprintLib::LogMessage(TEXT("Stopped recording thread"), TEXT(""), LogDebugLevel::Success); + finishing_signal_.signal(); } diff --git a/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.h b/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.h index 57d873e72e..538d23b9ab 100644 --- a/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.h +++ b/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.h @@ -10,6 +10,7 @@ #include #include "common/ClockFactory.hpp" #include "common/AirSimSettings.hpp" +#include "common/WorkerThread.hpp" class FRecordingThread : public FRunnable { @@ -19,9 +20,12 @@ class FRecordingThread : public FRunnable public: FRecordingThread(); virtual ~FRecordingThread(); + + static void init(); static void startRecording(const msr::airlib::ImageCaptureBase* camera, const msr::airlib::Kinematics::State* kinematics, const RecordingSetting& settings, msr::airlib::VehicleSimApiBase* vehicle_sim_api); - static void stopRecording(); + static void stopRecording(); + static void killRecording(); static bool isRecording(); protected: @@ -30,12 +34,14 @@ class FRecordingThread : public FRunnable virtual void Stop() override; virtual void Exit() override; -private: - void EnsureCompletion(); - private: FThreadSafeCounter stop_task_counter_; FRenderCommandFence read_pixel_fence_; + + static std::unique_ptr running_instance_; + static std::unique_ptr finishing_instance_; + static msr::airlib::WorkerThreadSignal finishing_signal_; + static bool first_; static std::unique_ptr instance_; diff --git a/Unreal/Plugins/AirSim/Source/SimMode/SimModeBase.cpp b/Unreal/Plugins/AirSim/Source/SimMode/SimModeBase.cpp index ae53d7272d..1ccb400b07 100644 --- a/Unreal/Plugins/AirSim/Source/SimMode/SimModeBase.cpp +++ b/Unreal/Plugins/AirSim/Source/SimMode/SimModeBase.cpp @@ -82,6 +82,7 @@ void ASimModeBase::BeginPlay() UAirBlueprintLib::LogMessage(TEXT("Press F1 to see help"), TEXT(""), LogDebugLevel::Informational); setupVehiclesAndCamera(); + FRecordingThread::init(); UWorld* World = GetWorld(); if (World) @@ -126,6 +127,7 @@ void ASimModeBase::setStencilIDs() void ASimModeBase::EndPlay(const EEndPlayReason::Type EndPlayReason) { FRecordingThread::stopRecording(); + FRecordingThread::killRecording(); world_sim_api_.reset(); api_provider_.reset(); api_server_.reset(); From 4f3d07896882c5b27ebf8321a35232efb13d534c Mon Sep 17 00:00:00 2001 From: Alexandr Yanenko Date: Tue, 15 Jan 2019 19:10:19 +0300 Subject: [PATCH 2/2] Fix 'killRecording' when recording is never started --- Unreal/Plugins/AirSim/Source/Recording/RecordingThread.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.cpp b/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.cpp index 9ec902c5d5..6fb24febf5 100644 --- a/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.cpp +++ b/Unreal/Plugins/AirSim/Source/Recording/RecordingThread.cpp @@ -72,6 +72,8 @@ void FRecordingThread::stopRecording() void FRecordingThread::killRecording() { + if (first_) return; + stopRecording(); bool finished = finishing_signal_.waitForRetry(1, 5); if (!finished) {