From d0cff9ca661d24eacd2d3c53e00e781e8eceb347 Mon Sep 17 00:00:00 2001 From: "Tikhomirova, Kseniya" Date: Mon, 21 Feb 2022 17:28:30 +0300 Subject: [PATCH 1/3] [SYCL] Fixes deadlock with PI traces when underlying modules called exit() If exit() is called by module inside pi call wrapped by mutex for pi traces RT hangs on tear down. Introduces mutex lock skip in this case. Signed-off-by: Tikhomirova, Kseniya --- sycl/source/detail/global_handler.cpp | 18 +++++++++++++++--- sycl/source/detail/global_handler.hpp | 2 +- sycl/source/detail/plugin.hpp | 12 +++++++++++- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/sycl/source/detail/global_handler.cpp b/sycl/source/detail/global_handler.cpp index c0174f81d6d80..13ce31f2c46b9 100644 --- a/sycl/source/detail/global_handler.cpp +++ b/sycl/source/detail/global_handler.cpp @@ -120,7 +120,7 @@ void GlobalHandler::registerDefaultContextReleaseHandler() { static DefaultContextReleaseHandler handler{}; } -void shutdown() { +void shutdown(bool NormalExit) { // Ensure neither host task is working so that no default context is accessed // upon its release if (GlobalHandler::instance().MHostTaskThreadPool.Inst) @@ -143,6 +143,10 @@ void shutdown() { // there's no need to load and unload plugins. if (GlobalHandler::instance().MPlugins.Inst) { for (plugin &Plugin : GlobalHandler::instance().getPlugins()) { + // shutdown() is called only when process is terminated by exitProcess() + // Emergency mode allows to skip some potentially unsafe code + if (!NormalExit) + Plugin.enableEmergencyMode(); // PluginParameter is reserved for future use that can control // some parameters in the plugin tear-down process. // Currently, it is not used. @@ -165,7 +169,7 @@ extern "C" __SYCL_EXPORT BOOL WINAPI DllMain(HINSTANCE hinstDLL, switch (fdwReason) { case DLL_PROCESS_DETACH: if (!lpReserved) - shutdown(); + shutdown(true); break; case DLL_PROCESS_ATTACH: case DLL_THREAD_ATTACH: @@ -175,11 +179,19 @@ extern "C" __SYCL_EXPORT BOOL WINAPI DllMain(HINSTANCE hinstDLL, return TRUE; // Successful DLL_PROCESS_ATTACH. } #else +void TearDown(int ExitCode, void *) { shutdown(ExitCode == EXIT_SUCCESS); } + +__attribute__((constructor(110))) static void syclLoad() { + int Res = on_exit(TearDown, nullptr); + if (Res != 0) { + exit(EXIT_FAILURE); + }; +} // Setting low priority on destructor ensures it runs after all other global // destructors. Priorities 0-100 are reserved by the compiler. The priority // value 110 allows SYCL users to run their destructors after runtime library // deinitialization. -__attribute__((destructor(110))) static void syclUnload() { shutdown(); } +//__attribute__((destructor(110))) static void syclUnload() { shutdown(); } #endif } // namespace detail } // namespace sycl diff --git a/sycl/source/detail/global_handler.hpp b/sycl/source/detail/global_handler.hpp index b3d6357686e0c..50614ad4e66e4 100644 --- a/sycl/source/detail/global_handler.hpp +++ b/sycl/source/detail/global_handler.hpp @@ -74,7 +74,7 @@ class GlobalHandler { private: friend void releaseDefaultContexts(); - friend void shutdown(); + friend void shutdown(bool NormalExit); // Constructor and destructor are declared out-of-line to allow incomplete // types as template arguments to unique_ptr. diff --git a/sycl/source/detail/plugin.hpp b/sycl/source/detail/plugin.hpp index 0a5851175711c..1c625600f19fe 100644 --- a/sycl/source/detail/plugin.hpp +++ b/sycl/source/detail/plugin.hpp @@ -163,7 +163,13 @@ class plugin { #endif RT::PiResult R; if (pi::trace(pi::TraceLevel::PI_TRACE_CALLS)) { - std::lock_guard Guard(*TracingMutex); + // MEmergencyModeEnabled signals if process is terminating now, it may + // happen by initiative from underlying modules and mutex can be still + // locked. Program behavior is undefined if thread terminates while owning + // a mutex so skip mutex ownership request in emergency mode. + auto Guard = MEmergencyModeEnabled + ? std::unique_lock() + : std::unique_lock(*TracingMutex); const char *FnName = PiCallInfo.getFuncName(); std::cout << "---> " << FnName << "(" << std::endl; RT::printArgs(Args...); @@ -245,6 +251,8 @@ class plugin { std::shared_ptr getPluginMutex() { return MPluginMutex; } + void enableEmergencyMode() { MEmergencyModeEnabled = true; } + private: std::shared_ptr MPlugin; backend MBackend; @@ -259,6 +267,8 @@ class plugin { // represents the unique ids of the last device of each platform // index of this vector corresponds to the index in PiPlatforms vector. std::vector LastDeviceIds; + + bool MEmergencyModeEnabled = false; }; // class plugin } // namespace detail } // namespace sycl From 116927f523d4dcffacdcfac67b671d686b8095f5 Mon Sep 17 00:00:00 2001 From: "Tikhomirova, Kseniya" Date: Tue, 22 Feb 2022 17:25:30 +0300 Subject: [PATCH 2/3] Restore global handler destruction logic and replace mutex with spin lock Signed-off-by: Tikhomirova, Kseniya --- sycl/source/detail/global_handler.cpp | 24 +++++++++--------------- sycl/source/detail/global_handler.hpp | 2 +- sycl/source/detail/plugin.hpp | 23 ++++++++++++----------- 3 files changed, 22 insertions(+), 27 deletions(-) diff --git a/sycl/source/detail/global_handler.cpp b/sycl/source/detail/global_handler.cpp index 13ce31f2c46b9..52443266e6e05 100644 --- a/sycl/source/detail/global_handler.cpp +++ b/sycl/source/detail/global_handler.cpp @@ -120,7 +120,7 @@ void GlobalHandler::registerDefaultContextReleaseHandler() { static DefaultContextReleaseHandler handler{}; } -void shutdown(bool NormalExit) { +void shutdown() { // Ensure neither host task is working so that no default context is accessed // upon its release if (GlobalHandler::instance().MHostTaskThreadPool.Inst) @@ -143,10 +143,12 @@ void shutdown(bool NormalExit) { // there's no need to load and unload plugins. if (GlobalHandler::instance().MPlugins.Inst) { for (plugin &Plugin : GlobalHandler::instance().getPlugins()) { - // shutdown() is called only when process is terminated by exitProcess() - // Emergency mode allows to skip some potentially unsafe code - if (!NormalExit) - Plugin.enableEmergencyMode(); + // shutdown() is called once and only when process is terminating. + // Till the time it is called all threads using RT must be closed so it + // should be safe to work will plugin without multi thread protection. + // Shutdown mode allows to skip some potentially unsafe code + // (lock/unlock). + Plugin.enableShutdownMode(); // PluginParameter is reserved for future use that can control // some parameters in the plugin tear-down process. // Currently, it is not used. @@ -169,7 +171,7 @@ extern "C" __SYCL_EXPORT BOOL WINAPI DllMain(HINSTANCE hinstDLL, switch (fdwReason) { case DLL_PROCESS_DETACH: if (!lpReserved) - shutdown(true); + shutdown(); break; case DLL_PROCESS_ATTACH: case DLL_THREAD_ATTACH: @@ -179,19 +181,11 @@ extern "C" __SYCL_EXPORT BOOL WINAPI DllMain(HINSTANCE hinstDLL, return TRUE; // Successful DLL_PROCESS_ATTACH. } #else -void TearDown(int ExitCode, void *) { shutdown(ExitCode == EXIT_SUCCESS); } - -__attribute__((constructor(110))) static void syclLoad() { - int Res = on_exit(TearDown, nullptr); - if (Res != 0) { - exit(EXIT_FAILURE); - }; -} // Setting low priority on destructor ensures it runs after all other global // destructors. Priorities 0-100 are reserved by the compiler. The priority // value 110 allows SYCL users to run their destructors after runtime library // deinitialization. -//__attribute__((destructor(110))) static void syclUnload() { shutdown(); } +__attribute__((destructor(110))) static void syclUnload() { shutdown(); } #endif } // namespace detail } // namespace sycl diff --git a/sycl/source/detail/global_handler.hpp b/sycl/source/detail/global_handler.hpp index 50614ad4e66e4..b3d6357686e0c 100644 --- a/sycl/source/detail/global_handler.hpp +++ b/sycl/source/detail/global_handler.hpp @@ -74,7 +74,7 @@ class GlobalHandler { private: friend void releaseDefaultContexts(); - friend void shutdown(bool NormalExit); + friend void shutdown(); // Constructor and destructor are declared out-of-line to allow incomplete // types as template arguments to unique_ptr. diff --git a/sycl/source/detail/plugin.hpp b/sycl/source/detail/plugin.hpp index 1c625600f19fe..ad566fd2126cc 100644 --- a/sycl/source/detail/plugin.hpp +++ b/sycl/source/detail/plugin.hpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -92,7 +93,7 @@ class plugin { plugin(const std::shared_ptr &Plugin, backend UseBackend, void *LibraryHandle) : MPlugin(Plugin), MBackend(UseBackend), MLibraryHandle(LibraryHandle), - TracingMutex(std::make_shared()), + MTracingSLock(std::make_shared()), MPluginMutex(std::make_shared()) {} plugin &operator=(const plugin &) = default; @@ -163,13 +164,13 @@ class plugin { #endif RT::PiResult R; if (pi::trace(pi::TraceLevel::PI_TRACE_CALLS)) { - // MEmergencyModeEnabled signals if process is terminating now, it may - // happen by initiative from underlying modules and mutex can be still - // locked. Program behavior is undefined if thread terminates while owning - // a mutex so skip mutex ownership request in emergency mode. - auto Guard = MEmergencyModeEnabled - ? std::unique_lock() - : std::unique_lock(*TracingMutex); + // MShutdownModeEnabled signals if process is terminating now. It covers + // normal case and undefined behavior when it happens by initiative from + // underlying modules (calling exit(1)). In case of unexpected exit(1) + // call sync primitive can be still locked. + auto Guard = MShutdownModeEnabled + ? std::unique_lock() + : std::unique_lock(*MTracingSLock); const char *FnName = PiCallInfo.getFuncName(); std::cout << "---> " << FnName << "(" << std::endl; RT::printArgs(Args...); @@ -251,13 +252,13 @@ class plugin { std::shared_ptr getPluginMutex() { return MPluginMutex; } - void enableEmergencyMode() { MEmergencyModeEnabled = true; } + void enableShutdownMode() { MShutdownModeEnabled = true; } private: std::shared_ptr MPlugin; backend MBackend; void *MLibraryHandle; // the handle returned from dlopen - std::shared_ptr TracingMutex; + std::shared_ptr MTracingSLock; // Mutex to guard PiPlatforms and LastDeviceIds. // Note that this is a temporary solution until we implement the global // Device/Platform cache later. @@ -268,7 +269,7 @@ class plugin { // index of this vector corresponds to the index in PiPlatforms vector. std::vector LastDeviceIds; - bool MEmergencyModeEnabled = false; + bool MShutdownModeEnabled = false; }; // class plugin } // namespace detail } // namespace sycl From 893787c6c9754a28d76f446ace3f89057e535af3 Mon Sep 17 00:00:00 2001 From: "Tikhomirova, Kseniya" Date: Tue, 22 Feb 2022 20:24:08 +0300 Subject: [PATCH 3/3] Fix typo Signed-off-by: Tikhomirova, Kseniya --- sycl/source/detail/global_handler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sycl/source/detail/global_handler.cpp b/sycl/source/detail/global_handler.cpp index 52443266e6e05..5fef7ea4e478b 100644 --- a/sycl/source/detail/global_handler.cpp +++ b/sycl/source/detail/global_handler.cpp @@ -145,7 +145,7 @@ void shutdown() { for (plugin &Plugin : GlobalHandler::instance().getPlugins()) { // shutdown() is called once and only when process is terminating. // Till the time it is called all threads using RT must be closed so it - // should be safe to work will plugin without multi thread protection. + // should be safe to work with plugin without multi thread protection. // Shutdown mode allows to skip some potentially unsafe code // (lock/unlock). Plugin.enableShutdownMode();