From 2fbacccaed04474b30abe98ccb161d8697d478b6 Mon Sep 17 00:00:00 2001
From: Sean Keely <Sean.Keely@amd.com>
Date: Wed, 6 May 2020 23:02:44 -0500
Subject: [PATCH] Correct handling of failed lazy_ptr constructors.

Contructor function must not be attempted twice even if the construction
attempt returns nullptr.

Change-Id: I75353e5e511769a96e4332f7f60887f6559c1cd5
---
 .../core/runtime/amd_gpu_agent.cpp            |  4 ++--
 runtime/hsa-runtime/core/util/lazy_ptr.h      | 20 ++++++++++++++-----
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp b/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp
index c6350024f7..33fbf67371 100644
--- a/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp
+++ b/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp
@@ -137,7 +137,7 @@ GpuAgent::GpuAgent(HSAuint32 node, const HsaNodeProperties& node_props)
 
 GpuAgent::~GpuAgent() {
   for (auto& blit : blits_) {
-    if (blit.created()) {
+    if (!blit.empty()) {
       hsa_status_t status = blit->Destroy(*this);
       assert(status == HSA_STATUS_SUCCESS);
     }
@@ -680,7 +680,7 @@ hsa_status_t GpuAgent::DmaFill(void* ptr, uint32_t value, size_t count) {
 
 hsa_status_t GpuAgent::EnableDmaProfiling(bool enable) {
   for (auto& blit : blits_) {
-    if (blit.created()) {
+    if (!blit.empty()) {
       const hsa_status_t stat = blit->EnableProfiling(enable);
       if (stat != HSA_STATUS_SUCCESS) {
         return stat;
diff --git a/runtime/hsa-runtime/core/util/lazy_ptr.h b/runtime/hsa-runtime/core/util/lazy_ptr.h
index dbeb1f7647..bb2583c714 100644
--- a/runtime/hsa-runtime/core/util/lazy_ptr.h
+++ b/runtime/hsa-runtime/core/util/lazy_ptr.h
@@ -87,6 +87,7 @@ template <typename T> class lazy_ptr {
 
   const std::unique_ptr<T>& operator->() const {
     make(true);
+    assert(obj != nullptr && "Null dereference through lazy_ptr.");
     return obj;
   }
 
@@ -107,7 +108,17 @@ template <typename T> class lazy_ptr {
   void touch() const { make(false); }
 
   // Tells if the lazy object has been constructed or not.
-  bool created() const { return obj != nullptr; }
+  // Construction may fail silently (return nullptr).
+  bool created() const {
+    std::atomic_thread_fence(std::memory_order_acquire);
+    return func == nullptr;
+  }
+
+  // Tells if the lazy object exists or not.
+  bool empty() const {
+    std::atomic_thread_fence(std::memory_order_acquire);
+    return obj == nullptr;
+  }
 
  private:
   mutable std::unique_ptr<T> obj;
@@ -122,16 +133,15 @@ template <typename T> class lazy_ptr {
       return;
     }
     MAKE_SCOPE_GUARD([&]() { lock.Release(); });
-    if (obj != nullptr) return;
+    if (func == nullptr) return;
     T* ptr = func();
-    std::atomic_thread_fence(std::memory_order_release);
     obj.reset(ptr);
+    std::atomic_thread_fence(std::memory_order_release);
     func = nullptr;
   }
 
   __forceinline void make(bool block) const {
-    std::atomic_thread_fence(std::memory_order_acquire);
-    if (obj == nullptr) {
+    if (!created()) {
       make_body(block);
     }
   }