[SYCL] Fix deadlock in Scheduler on Windows (#1703)

There was a deadlock in Scheduler on Windows only. This deadlock happens in implementation of std::shared_timed_mutex of MSVC's C++ std library.
The deadlock on Windows happens because lock ordering is not specified in C++ spec when we do lock and lock_shared simultaneously, and lock and lock_shared both wait. 
It can be fixed in 2 ways:

1. upgrade DPC++ runtime to C++17 and change std::shared_timed_mutex to std::shared_mutex, there will be no deadlock on Windows out of the box. No regression on Linux.
2. implement spinlock

We can't upgrade to C++17 yet, so spinlock was implemented. It fixed the deadlock on Windows. But it creates another deadlock, this time on Linux only in implementation of lock and shared_lock in pthread. To fix this, added #ifdef _WIN32 to separate Windows and Linux parts of code.

Signed-off-by: Dmitry Vodopyanov <[email protected]>

Author: dm-vodopyanov

sycl/source/detail/scheduler/scheduler.cpp

@@ -66,7 +66,8 @@ EventImplPtr Scheduler::addCG(std::unique_ptr<detail::CG> CommandGroup,
   Command *NewCmd = nullptr;
   const bool IsKernel = CommandGroup->getType() == CG::KERNEL;
   {
-    std::lock_guard<std::shared_timed_mutex> Lock(MGraphLock);
+    std::unique_lock<std::shared_timed_mutex> Lock(MGraphLock, std::defer_lock);
+    lockSharedTimedMutex(Lock);
 
     switch (CommandGroup->getType()) {
     case CG::UPDATE_HOST:
@@ -98,7 +99,8 @@ EventImplPtr Scheduler::addCG(std::unique_ptr<detail::CG> CommandGroup,
 }
 
 EventImplPtr Scheduler::addCopyBack(Requirement *Req) {
-  std::lock_guard<std::shared_timed_mutex> Lock(MGraphLock);
+  std::unique_lock<std::shared_timed_mutex> Lock(MGraphLock, std::defer_lock);
+  lockSharedTimedMutex(Lock);
   Command *NewCmd = MGraphBuilder.addCopyBack(Req);
   // Command was not creted because there were no operations with
   // buffer.
@@ -154,7 +156,8 @@ void Scheduler::cleanupFinishedCommands(EventImplPtr FinishedEvent) {
 }
 
 void Scheduler::removeMemoryObject(detail::SYCLMemObjI *MemObj) {
-  std::lock_guard<std::shared_timed_mutex> Lock(MGraphLock);
+  std::unique_lock<std::shared_timed_mutex> Lock(MGraphLock, std::defer_lock);
+  lockSharedTimedMutex(Lock);
 
   MemObjRecord *Record = MGraphBuilder.getMemObjRecord(MemObj);
   if (!Record)
@@ -169,7 +172,8 @@ void Scheduler::removeMemoryObject(detail::SYCLMemObjI *MemObj) {
 
 EventImplPtr Scheduler::addHostAccessor(Requirement *Req,
                                         const bool destructor) {
-  std::lock_guard<std::shared_timed_mutex> Lock(MGraphLock);
+  std::unique_lock<std::shared_timed_mutex> Lock(MGraphLock, std::defer_lock);
+  lockSharedTimedMutex(Lock);
 
   Command *NewCmd = MGraphBuilder.addHostAccessor(Req, destructor);
 
@@ -216,6 +220,25 @@ Scheduler::Scheduler() {
                      QueueOrder::Ordered, /*PropList=*/{}));
 }
 
+void Scheduler::lockSharedTimedMutex(
+    std::unique_lock<std::shared_timed_mutex> &Lock) {
+#ifdef _WIN32
+  // Avoiding deadlock situation for MSVC. std::shared_timed_mutex specification
+  // does not specify a priority for shared and exclusive accesses. It will be a
+  // deadlock in MSVC's std::shared_timed_mutex implementation, if exclusive
+  // access occurs after shared access.
+  // TODO: after switching to C++17, change std::shared_timed_mutex to
+  // std::shared_mutex and use std::lock_guard here both for Windows and Linux.
+  while (!Lock.owns_lock()) {
+    Lock.try_lock();
+  }
+#else
+  // It is a deadlock on UNIX in implementation of lock and lock_shared, if
+  // try_lock in the loop above will be executed, so using a single lock here
+  Lock.lock();
+#endif // _WIN32
+}
+
 } // namespace detail
 } // namespace sycl
 } // __SYCL_INLINE_NAMESPACE(cl)

sycl/source/detail/scheduler/scheduler.hpp

@@ -430,6 +430,13 @@ class Scheduler {
   Scheduler();
   static Scheduler instance;
 
+  /// Provides exclusive access to std::shared_timed_mutex object with deadlock
+  /// avoidance
+  ///
+  /// \param Lock is an instance of std::unique_lock<std::shared_timed_mutex>
+  /// class
+  void lockSharedTimedMutex(std::unique_lock<std::shared_timed_mutex> &Lock);
+
   static void enqueueLeavesOfReqUnlocked(const Requirement *const Req);
 
   /// Graph builder class.
@@ -687,6 +694,8 @@ class Scheduler {
   void waitForRecordToFinish(MemObjRecord *Record);
 
   GraphBuilder MGraphBuilder;
+  // TODO: after switching to C++17, change std::shared_timed_mutex to
+  // std::shared_mutex
   std::shared_timed_mutex MGraphLock;
 
   QueueImplPtr DefaultHostQueue;