[Openmp-commits] [PATCH] D136952: [libomptarget] Fix a race condition in checkDeviceAndCtors

[Lechen Yu via Phabricator via Openmp-commits]

lechenyu created this revision.
lechenyu added reviewers: jdoerfert, tianshilei1992, jhuber6.
lechenyu added a project: OpenMP.
Herald added a project: All.
lechenyu requested review of this revision.
Herald added subscribers: openmp-commits, sstefan1.

When multiple threads invoke `checkDeviceAndCtors`, both of them may read `true` from the shared variable `Device.HasPendingGlobals`, and then invoke `initLibrary` redundantly. Therefore only protecting the access to `Device.HasPendingGlobals` is not sufficient to guarantee that `initLibrary` is invoked just once.

To fix this race condition, we move the invocation of `initLibrary` into the critical section, and remove the same lock inside `initLibrary`,


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136952

Files:
  openmp/libomptarget/src/omptarget.cpp


Index: openmp/libomptarget/src/omptarget.cpp
===================================================================
--- openmp/libomptarget/src/omptarget.cpp
+++ openmp/libomptarget/src/omptarget.cpp
@@ -80,9 +80,6 @@
   int Rc = OFFLOAD_SUCCESS;
   bool SupportsEmptyImages = Device.RTL->supports_empty_images &&
                              Device.RTL->supports_empty_images() > 0;
-
-  std::lock_guard<decltype(Device.PendingGlobalsMtx)> LG(
-      Device.PendingGlobalsMtx);
   {
     std::lock_guard<decltype(PM->TrlTblMtx)> LG(PM->TrlTblMtx);
     for (auto *HostEntriesBegin : PM->HostEntriesBeginRegistrationOrder) {
@@ -320,16 +317,14 @@
   DeviceTy &Device = *PM->Devices[DeviceID];
 
   // Check whether global data has been mapped for this device
-  bool HasPendingGlobals;
   {
     std::lock_guard<decltype(Device.PendingGlobalsMtx)> LG(
         Device.PendingGlobalsMtx);
-    HasPendingGlobals = Device.HasPendingGlobals;
-  }
-  if (HasPendingGlobals && initLibrary(Device) != OFFLOAD_SUCCESS) {
-    REPORT("Failed to init globals on device %" PRId64 "\n", DeviceID);
-    handleTargetOutcome(false, Loc);
-    return true;
+    if (Device.HasPendingGlobals && initLibrary(Device) != OFFLOAD_SUCCESS) {
+      REPORT("Failed to init globals on device %" PRId64 "\n", DeviceID);
+      handleTargetOutcome(false, Loc);
+      return true;
+    }
   }
 
   return false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136952.471546.patch
Type: text/x-patch
Size: 1386 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20221028/d714786c/attachment.bin>