[SYCL] Avoid datarace when waiting on the same event in multiple threads (#1803)

One thread could acquire shared access in `waitForEvent` after another
thread executed `cleanupFinishedCommands` (which has exclusive access).
This resulted in "Event has no associated command?" assertion failure
in `GraphProcessor::waitForEvent`.

Signed-off-by: Dmitri Mokhov <[email protected]>

Author: dnmokhov

sycl/source/detail/event_impl.hpp

@@ -133,11 +133,15 @@ class event_impl {
 
   /// Returns command that is associated with the event.
   ///
+  /// Scheduler mutex must be locked in read mode when this is called.
+  ///
   /// @return a generic pointer to Command object instance.
   void *getCommand() { return MCommand; }
 
   /// Associates this event with the command.
   ///
+  /// Scheduler mutex must be locked in write mode when this is called.
+  ///
   /// @param Command is a generic pointer to Command object instance.
   void setCommand(void *Command) { MCommand = Command; }
 

sycl/source/detail/scheduler/graph_processor.cpp

@@ -38,7 +38,11 @@ Scheduler::GraphProcessor::getWaitList(EventImplPtr Event) {
 
 void Scheduler::GraphProcessor::waitForEvent(EventImplPtr Event) {
   Command *Cmd = getCommand(Event);
-  assert(Cmd && "Event has no associated command?");
+  // Command can be nullptr if user creates cl::sycl::event explicitly or the
+  // event has been waited on by another thread
+  if (!Cmd)
+    return;
+
   EnqueueResultT Res;
   bool Enqueued = enqueueCommand(Cmd, Res, BLOCKING);
   if (!Enqueued && EnqueueResultT::SyclEnqueueFailed == Res.MResult)

sycl/unittests/scheduler/CMakeLists.txt

@@ -5,5 +5,6 @@ add_sycl_unittest(SchedulerTests OBJECT
     LeafLimit.cpp
     MemObjCommandCleanup.cpp
     CommandsWaitForEvents.cpp
+    WaitAfterCleanup.cpp
     utils.cpp
 )

sycl/unittests/scheduler/WaitAfterCleanup.cpp

@@ -0,0 +1,28 @@
+//==------------ WaitAfterCleanup.cpp ---- Scheduler unit tests ------------==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SchedulerTest.hpp"
+#include "SchedulerTestUtils.hpp"
+
+using namespace cl::sycl;
+
+TEST_F(SchedulerTest, WaitAfterCleanup) {
+  auto Cmd = new MockCommand(detail::getSyclObjImpl(MQueue));
+  auto Event = Cmd->getEvent();
+  ASSERT_NE(Event, nullptr) << "Command must have an event\n";
+
+  detail::Scheduler::getInstance().waitForEvent(Event);
+  ASSERT_EQ(Event->getCommand(), Cmd)
+      << "Command should not have been cleaned up yet\n";
+
+  detail::Scheduler::getInstance().cleanupFinishedCommands(Event);
+  ASSERT_EQ(Event->getCommand(), nullptr)
+      << "Command should have been cleaned up\n";
+
+  detail::Scheduler::getInstance().waitForEvent(Event);
+}