From 6b361bc1a0ca44dbd4efab1eada9b931b9eb82c1 Mon Sep 17 00:00:00 2001 From: Christophe Paquot Date: Mon, 11 May 2020 16:35:13 -0700 Subject: [PATCH] HPC : Intermittent hangs are observed while running Gromacs benchmarks SWDEV-235579 Move the lock before destroying the queue as there's a multithreaded race condition if the queue is being destroy and right after we set queue_ to nullptr, another thread can call ihipWaitStreams which will then call create on that same stream because queue is now nullptr. Moving the lock on streamSet prevents this from happening because we would remove the stream from that list and therefore ihipWait will not try to call asHostQueue which tries to create the queue if not created yet since the stream won't be in the list anymore Change-Id: I3108657ab403d39d4123e83294fcf1f0880e5563 --- rocclr/hip_stream.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rocclr/hip_stream.cpp b/rocclr/hip_stream.cpp index e4bf4fe192..3bd7d343f7 100644 --- a/rocclr/hip_stream.cpp +++ b/rocclr/hip_stream.cpp @@ -23,7 +23,7 @@ #include "hip_event.hpp" #include "thread/monitor.hpp" -static amd::Monitor streamSetLock("Guards global stream set"); +static amd::Monitor streamSetLock{"Guards global stream set"}; static std::unordered_set streamSet; // Internal structure for stream callback handler @@ -83,11 +83,11 @@ amd::HostQueue* Stream::asHostQueue(bool skip_alloc) { // ================================================================================================ void Stream::Destroy() { if (queue_ != nullptr) { - queue_->release(); - queue_ = nullptr; - amd::ScopedLock lock(streamSetLock); streamSet.erase(this); + + queue_->release(); + queue_ = nullptr; } delete this; }