Merge pull request #175 from LLNL/bugfix/burmark1/get_a_race

MrBurmark · web-flow · commit 8c3a0cebd2b2 · 2025-08-28T14:47:47.000-07:00
Fix race conditions in resources get_a_* functions
diff --git a/include/camp/resource/cuda.hpp b/include/camp/resource/cuda.hpp
@@ -17,6 +17,7 @@
 #include "camp/resource/platform.hpp"
 
 #include <cuda_runtime.h>
+#include <array>
 #include <mutex>
 
 namespace camp
@@ -81,28 +82,25 @@ namespace resources
     {
       static cudaStream_t get_a_stream(int num)
       {
-        static cudaStream_t streams[16] = {};
-        static int previous = 0;
-
-        static std::once_flag m_onceFlag;
-        static std::mutex m_mtx;
-
-        std::call_once(m_onceFlag, [] {
-          if (streams[0] == nullptr) {
-            for (auto &s : streams) {
-              campCudaErrchkDiscardReturn(cudaStreamCreate(&s));
-            }
-          }
-        });
+        static constexpr int num_streams = 16;
+        static std::array<cudaStream_t, num_streams> s_streams = [] {
+              std::array<cudaStream_t, num_streams> streams;
+              for (auto &s : streams) {
+                campCudaErrchkDiscardReturn(cudaStreamCreate(&s));
+              }
+              return streams;
+            }();
+
+        static std::mutex s_mtx;
+        static int s_previous = num_streams-1;
 
         if (num < 0) {
-          m_mtx.lock();
-          previous = (previous + 1) % 16;
-          m_mtx.unlock();
-          return streams[previous];
+          std::lock_guard<std::mutex> lock(s_mtx);
+          s_previous = (s_previous + 1) % num_streams;
+          return s_streams[s_previous];
         }
 
-        return streams[num % 16];
+        return s_streams[num % num_streams];
       }
 
       // Private from-stream constructor
diff --git a/include/camp/resource/hip.hpp b/include/camp/resource/hip.hpp
@@ -16,6 +16,7 @@
 #include "camp/resource/platform.hpp"
 
 #include <hip/hip_runtime.h>
+#include <array>
 #include <mutex>
 
 namespace camp
@@ -78,28 +79,25 @@ namespace resources
     {
       static hipStream_t get_a_stream(int num)
       {
-        static hipStream_t streams[16] = {};
-        static int previous = 0;
-
-        static std::once_flag m_onceFlag;
-        static std::mutex m_mtx;
-
-        std::call_once(m_onceFlag, [] {
-          if (streams[0] == nullptr) {
-            for (auto &s : streams) {
-              campHipErrchkDiscardReturn(hipStreamCreate(&s));
-            }
-          }
-        });
+        static constexpr int num_streams = 16;
+        static std::array<hipStream_t, num_streams> s_streams = [] {
+              std::array<hipStream_t, num_streams> streams;
+              for (auto &s : streams) {
+                campHipErrchkDiscardReturn(hipStreamCreate(&s));
+              }
+              return streams;
+            }();
+
+        static std::mutex s_mtx;
+        static int s_previous = num_streams-1;
 
         if (num < 0) {
-          m_mtx.lock();
-          previous = (previous + 1) % 16;
-          m_mtx.unlock();
-          return streams[previous];
+          std::lock_guard<std::mutex> lock(s_mtx);
+          s_previous = (s_previous + 1) % num_streams;
+          return s_streams[s_previous];
         }
 
-        return streams[num % 16];
+        return s_streams[num % num_streams];
       }
 
       // Private from-stream constructor
diff --git a/include/camp/resource/omp_target.hpp b/include/camp/resource/omp_target.hpp
@@ -63,19 +63,19 @@ namespace resources
     {
       static char *get_addr(int num)
       {
-        static char addrs[16] = {};
-        static int previous = 0;
+        static constexpr int num_addrs = 16;
+        static char s_addrs[num_addrs] = {};
 
-        static std::mutex m_mtx;
+        static std::mutex s_mtx;
+        static int s_previous = num_addrs-1;
 
         if (num < 0) {
-          m_mtx.lock();
-          previous = (previous + 1) % 16;
-          m_mtx.unlock();
-          return &addrs[previous];
+          std::lock_guard<std::mutex> lock(s_mtx);
+          s_previous = (s_previous + 1) % num_addrs;
+          return &s_addrs[s_previous];
         }
 
-        return &addrs[num % 16];
+        return &s_addrs[num % num_addrs];
       }
 
       // Private from-address constructor
diff --git a/include/camp/resource/sycl.hpp b/include/camp/resource/sycl.hpp
@@ -44,94 +44,162 @@ namespace resources
 
     class Sycl
     {
-      static sycl::queue *get_a_queue(sycl::context &syclContext,
-                                      int num,
-                                      bool useContext)
-      {
-        constexpr auto gpuSelector = sycl::gpu_selector_v;
-        static sycl::property_list propertyList =
-            sycl::property_list(sycl::property::queue::in_order());
-        static sycl::context privateContext;
-        static sycl::context *contextInUse = NULL;
-        static std::map<sycl::context *, std::array<sycl::queue, 16>> queueMap;
-
-
-        static std::mutex m_mtx;
-        m_mtx.lock();
-
-        // User passed a context, use it
-        if (useContext) {
-          contextInUse = &syclContext;
-
-          if (queueMap.find(contextInUse) == queueMap.end()) {
-            queueMap[contextInUse] = {
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList)};
-          }
-        } else {  // User did not pass context, use last used or private one
-          if (contextInUse == NULL) {
-            contextInUse = &privateContext;
-            queueMap[contextInUse] = {
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList),
-                sycl::queue(*contextInUse, gpuSelector, propertyList)};
-          }
+      /*
+       * \brief Get the camp managed sycl context.
+       *
+       * Note that the first call sets up the context with the given argument.
+       *
+       * \return Reference to the camp managed sycl context.
+       */
+      static sycl::context& get_private_context(const sycl::context* syclContext)
+      {
+        static sycl::context s_context(syclContext ? *syclContext : sycl::context());
+        return s_context;
+      }
+
+      /*
+       * \brief Get the per thread camp managed sycl context.
+       *
+       * Note that the first call sets up the context with the given argument.
+       *
+       * \return Reference to the per thread camp managed sycl context.
+       */
+      static sycl::context& get_thread_private_context(sycl::context const& syclContext)
+      {
+        thread_local sycl::context t_context(syclContext);
+        return t_context;
+      }
+
+      /*
+       * \brief Get the per thread camp managed sycl context.
+       *
+       * Note that the first call sets up the context with the given argument.
+       *
+       * \return Reference to the per thread camp managed sycl context.
+       */
+      static sycl::context const& get_thread_default_context(sycl::context const& syclContext)
+      {
+        get_private_context(&syclContext);
+        return get_thread_private_context(syclContext);
+      }
+
+public:
+      /*
+       * \brief Get the camp managed sycl context.
+       *
+       * \return Const reference to the camp managed sycl context.
+       */
+      static sycl::context const& get_default_context()
+      {
+        return get_private_context(nullptr);
+      }
+
+      /*
+       * \brief Get the per thread camp managed sycl context.
+       *
+       * \return Const reference to the per thread camp managed sycl context.
+       */
+      static sycl::context const& get_thread_default_context()
+      {
+        return get_thread_private_context(get_private_context(nullptr));
+      }
+
+      /*
+       * \brief Set the camp managed sycl context.
+       */
+      static void set_default_context(sycl::context const& syclContext)
+      {
+        get_private_context(&syclContext) = syclContext;
+      }
+
+      /*
+       * \brief Set the per thread camp managed sycl context.
+       */
+      static void set_thread_default_context(sycl::context const& syclContext)
+      {
+        get_private_context(&syclContext);
+        get_thread_private_context(syclContext) = syclContext;
+      }
+
+private:
+      static sycl::queue *get_a_queue(const sycl::context* syclContext,
+                                      int num)
+      {
+        static constexpr int num_queues = 16;
+
+        static std::mutex s_mtx;
+
+        // note that this type must not invalidate iterators when modified
+        using value_second_type = std::pair<int, std::array<sycl::queue, num_queues>>;
+        using queueMap_type = std::map<const sycl::context*, value_second_type>;
+        static queueMap_type queueMap;
+        static const typename queueMap_type::iterator queueMap_end = queueMap.end();
+        thread_local typename queueMap_type::iterator cachedContextIter = queueMap_end;
+
+        if (syclContext) {
+          // implement sticky contexts
+          set_thread_default_context(*syclContext);
+        }
+        syclContext = &get_thread_default_context();
+
+        if (syclContext != cachedContextIter->first) {
+          cachedContextIter = queueMap_end;
         }
-        m_mtx.unlock();
 
-        static int previous = 0;
+        if (cachedContextIter == queueMap_end || num < 0) {
+          std::lock_guard<std::mutex> lock(s_mtx);
+
+          if (cachedContextIter == queueMap_end) {
+            cachedContextIter = queueMap.find(syclContext);
+            if (cachedContextIter == queueMap_end) {
+              static constexpr auto gpuSelector = sycl::gpu_selector_v;
+              static const sycl::property_list propertyList =
+                  sycl::property_list(sycl::property::queue::in_order());
+
+              cachedContextIter = queueMap.emplace(syclContext,
+                  value_second_type(num_queues-1, {
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList),
+                    sycl::queue(*syclContext, gpuSelector, propertyList)})
+                  ).first;
+            }
+          }
 
-        static std::once_flag m_onceFlag;
-        CAMP_ALLOW_UNUSED_LOCAL(m_onceFlag);
-        if (num < 0) {
-          m_mtx.lock();
-          previous = (previous + 1) % 16;
-          m_mtx.unlock();
-          return &queueMap[contextInUse][previous];
+          if (num < 0) {
+            int& previous = cachedContextIter->second.first;
+            previous = (previous + 1) % num_queues;
+            return &cachedContextIter->second.second[previous];
+          }
         }
 
-        return &queueMap[contextInUse][num % 16];
+        return &cachedContextIter->second.second[num % num_queues];
       }
 
       // Private from-queue constructor
       Sycl(sycl::queue& q) : qu(&q) {}
 
     public:
-      Sycl(int group = -1)
+      Sycl(int group = -1, sycl::context const& syclContext = get_thread_default_context())
+          : qu(get_a_queue(&syclContext, group))
       {
-        sycl::context temp;
-        qu = get_a_queue(temp, group, false);
       }
 
-      Sycl(sycl::context &syclContext, int group = -1)
-          : qu(get_a_queue(syclContext, group, true))
+      [[deprecated]]
+      Sycl(sycl::context const& syclContext, int group = -1)
+          : qu(get_a_queue(&syclContext, group))
       {
       }
 
@@ -141,13 +209,16 @@ namespace resources
         return Sycl(q);
       }
 
-      // Methods
-      Platform get_platform() const { return Platform::sycl; }
+      // get default resource
       static Sycl get_default()
       {
-        static Sycl h;
-        return h;
+        return Sycl(0, get_default_context());
       }
+
+      // Methods
+      Platform get_platform() const { return Platform::sycl; }
+
+      // Event
       SyclEvent get_event() { return SyclEvent(get_queue()); }
       Event get_event_erased() { return Event{SyclEvent(get_queue())}; }
       void wait() { qu->wait(); }
@@ -207,6 +278,7 @@ namespace resources
         }
       }
 
+      // implementation specific
       sycl::queue *get_queue() { return qu; }
       sycl::queue const *get_queue() const { return qu; }
 

Original file line number	Diff line number	Diff line change
`@@ -63,19 +63,19 @@ namespace resources`
`63`	`63`	`{`
`64`	`64`	`static char *get_addr(int num)`
`65`	`65`	`{`
`66`		`- static char addrs[16] = {};`
`67`		`- static int previous = 0;`
	`66`	`+ static constexpr int num_addrs = 16;`
	`67`	`+ static char s_addrs[num_addrs] = {};`
`68`	`68`
`69`		`- static std::mutex m_mtx;`
	`69`	`+ static std::mutex s_mtx;`
	`70`	`+ static int s_previous = num_addrs-1;`
`70`	`71`
`71`	`72`	`if (num < 0) {`
`72`		`- m_mtx.lock();`
`73`		`- previous = (previous + 1) % 16;`
`74`		`- m_mtx.unlock();`
`75`		`- return &addrs[previous];`
	`73`	`+ std::lock_guard<std::mutex> lock(s_mtx);`
	`74`	`+ s_previous = (s_previous + 1) % num_addrs;`
	`75`	`+ return &s_addrs[s_previous];`
`76`	`76`	`}`
`77`	`77`
`78`		`- return &addrs[num % 16];`
	`78`	`+ return &s_addrs[num % num_addrs];`
`79`	`79`	`}`
`80`	`80`
`81`	`81`	`// Private from-address constructor`