use release for cl-objects instead of delete

- fix for data race in events
- modification of the addition child event

Change-Id: I6ea3a413f13f13a91d37d20d8b9fad37d0ffafb9

Author: kamdiedrich

runtime/command_queue/command_queue.cpp

@@ -76,7 +76,6 @@ CommandQueue::CommandQueue(Context *context, Device *deviceId, const cl_queue_pr
 CommandQueue::~CommandQueue() {
     if (virtualEvent) {
         UNRECOVERABLE_IF(this->virtualEvent->getCommandQueue() != this && this->virtualEvent->getCommandQueue() != nullptr);
-        virtualEvent->setCurrentCmdQVirtualEvent(false);
         virtualEvent->decRefInternal();
     }
 
@@ -519,7 +518,6 @@ void CommandQueue::enqueueBlockedMapUnmapOperation(const cl_event *eventWaitList
     eventBuilder->finalize();
 
     if (this->virtualEvent) {
-        this->virtualEvent->setCurrentCmdQVirtualEvent(false);
         this->virtualEvent->decRefInternal();
     }
     this->virtualEvent = eventBuilder->getEvent();

runtime/command_queue/command_queue.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Intel Corporation
+ * Copyright (C) 2017-2019 Intel Corporation
  *
  * SPDX-License-Identifier: MIT
  *
@@ -10,6 +10,7 @@
 #include "runtime/helpers/engine_control.h"
 #include "runtime/helpers/task_information.h"
 #include "runtime/helpers/dispatch_info.h"
+#include "runtime/event/user_event.h"
 #include "instrumentation.h"
 #include <atomic>
 #include <cstdint>
@@ -341,6 +342,13 @@ class CommandQueue : public BaseObject<_cl_command_queue> {
 
     MOCKABLE_VIRTUAL void releaseIndirectHeap(IndirectHeap::Type heapType);
 
+    void releaseVirtualEvent() {
+        if (this->virtualEvent != nullptr) {
+            this->virtualEvent->decRefInternal();
+            this->virtualEvent = nullptr;
+        }
+    }
+
     cl_command_queue_properties getCommandQueueProperties() const {
         return commandQueueProperties;
     }

runtime/command_queue/enqueue_common.h

@@ -659,7 +659,6 @@ void CommandQueueHw<GfxFamily>::enqueueBlocked(
         eventBuilder = &internalEventBuilder;
         DBG_LOG(EventsDebugEnable, "enqueueBlocked", "new virtualEvent", eventBuilder->getEvent());
     }
-    eventBuilder->getEvent()->setCurrentCmdQVirtualEvent(true);
 
     //update queue taskCount
     taskCount = eventBuilder->getEvent()->getCompletionStamp();
@@ -720,7 +719,6 @@ void CommandQueueHw<GfxFamily>::enqueueBlocked(
     eventBuilder->finalize();
 
     if (this->virtualEvent) {
-        this->virtualEvent->setCurrentCmdQVirtualEvent(false);
         this->virtualEvent->decRefInternal();
     }
 

runtime/event/event.cpp

@@ -416,11 +416,6 @@ void Event::unblockEventsBlockedByThis(int32_t transitionStatus) {
 
         childEvent->unblockEventBy(*this, taskLevelToPropagate, transitionStatus);
 
-        if (childEvent->getCommandQueue() && childEvent->isCurrentCmdQVirtualEvent()) {
-            // Check virtual event state and delete it if possible.
-            childEvent->getCommandQueue()->isQueueBlocked();
-        }
-
         childEvent->decRefInternal();
         auto next = childEventRef->next;
         delete childEventRef;

runtime/event/event.h

@@ -203,6 +203,10 @@ class Event : public BaseObject<_cl_event>, public IDNode<Event> {
         }
     }
 
+    bool peekIsCmdSubmitted() {
+        return submittedCmd != nullptr;
+    }
+
     //commands blocked by user event depencies
     bool isReadyForSubmission();
 
@@ -217,6 +221,10 @@ class Event : public BaseObject<_cl_event>, public IDNode<Event> {
         return (CL_COMMAND_USER == cmdType);
     }
 
+    bool isEventWithoutCommand() {
+        return eventWithoutCommand;
+    }
+
     Context *getContext() {
         return ctx;
     }

runtime/event/event_builder.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Intel Corporation
+ * Copyright (C) 2017-2019 Intel Corporation
  *
  * SPDX-License-Identifier: MIT
  *
@@ -53,7 +53,8 @@ void EventBuilder::finalize() {
 
             //do not add as child if:
             //parent has no parents and is not blocked
-            if (!(parent->peekIsBlocked() == false && parent->taskLevel != Event::eventNotReady)) {
+            if (!(parent->peekIsBlocked() == false && parent->taskLevel != Event::eventNotReady) ||
+                (!parent->isEventWithoutCommand() && !parent->peekIsCmdSubmitted())) {
                 parent->addChild(*this->event);
             }
         }

runtime/helpers/queue_helpers.h

@@ -11,6 +11,16 @@
 #include "runtime/helpers/get_info.h"
 
 namespace OCLRT {
+
+inline void releaseVirtualEvent(CommandQueue &commandQueue) {
+    if (commandQueue.getRefApiCount() == 1) {
+        commandQueue.releaseVirtualEvent();
+    }
+}
+
+inline void releaseVirtualEvent(DeviceQueue &commandQueue) {
+}
+
 template <typename QueueType>
 void retainQueue(cl_command_queue commandQueue, cl_int &retVal) {
     using BaseType = typename QueueType::BaseType;
@@ -26,6 +36,7 @@ void releaseQueue(cl_command_queue commandQueue, cl_int &retVal) {
     using BaseType = typename QueueType::BaseType;
     auto queue = castToObject<QueueType>(static_cast<BaseType *>(commandQueue));
     if (queue) {
+        releaseVirtualEvent(*queue);
         queue->release();
         retVal = CL_SUCCESS;
     }

unit_tests/command_queue/command_queue_hw_tests.cpp

@@ -11,6 +11,7 @@
 #include "unit_tests/fixtures/buffer_fixture.h"
 #include "unit_tests/fixtures/context_fixture.h"
 #include "unit_tests/fixtures/device_fixture.h"
+#include "unit_tests/utilities/base_object_utils.h"
 #include "unit_tests/helpers/debug_manager_state_restore.h"
 #include "unit_tests/helpers/unit_test_helper.h"
 #include "unit_tests/mocks/mock_buffer.h"
@@ -361,6 +362,7 @@ HWTEST_F(CommandQueueHwTest, GivenNotCompleteUserEventPassedToEnqueueWhenEventIs
     mockCSR->getMemoryManager()->freeGraphicsMemory(privateSurface);
     mockCSR->getMemoryManager()->freeGraphicsMemory(printfSurface);
     mockCSR->getMemoryManager()->freeGraphicsMemory(constantSurface);
+    pCmdQ->isQueueBlocked();
 }
 
 typedef CommandQueueHwTest BlockedCommandQueueTest;
@@ -387,6 +389,8 @@ HWTEST_F(BlockedCommandQueueTest, givenCommandQueueWhenBlockedCommandIsBeingSubm
     EXPECT_EQ(0u, ioh.getUsed());
     EXPECT_EQ(0u, dsh.getUsed());
     EXPECT_EQ(defaultSshUse, ssh.getUsed());
+
+    pCmdQ->isQueueBlocked();
 }
 
 HWTEST_F(BlockedCommandQueueTest, givenCommandQueueWithUsedHeapsWhenBlockedCommandIsBeingSubmittedThenQueueHeapsAreNotUsed) {
@@ -417,6 +421,8 @@ HWTEST_F(BlockedCommandQueueTest, givenCommandQueueWithUsedHeapsWhenBlockedComma
     EXPECT_EQ(spaceToUse, ioh.getUsed());
     EXPECT_EQ(spaceToUse, dsh.getUsed());
     EXPECT_EQ(sshSpaceUse, ssh.getUsed());
+
+    pCmdQ->isQueueBlocked();
 }
 
 HWTEST_F(BlockedCommandQueueTest, givenCommandQueueWhichHasSomeUnusedHeapsWhenBlockedCommandIsBeingSubmittedThenThoseHeapsAreBeingUsed) {
@@ -443,6 +449,8 @@ HWTEST_F(BlockedCommandQueueTest, givenCommandQueueWhichHasSomeUnusedHeapsWhenBl
     EXPECT_EQ(iohBase, ioh.getCpuBase());
     EXPECT_EQ(dshBase, dsh.getCpuBase());
     EXPECT_EQ(sshBase, ssh.getCpuBase());
+
+    pCmdQ->isQueueBlocked();
 }
 
 HWTEST_F(BlockedCommandQueueTest, givenEnqueueBlockedByUserEventWhenItIsEnqueuedThenKernelReferenceCountIsIncreased) {
@@ -459,6 +467,7 @@ HWTEST_F(BlockedCommandQueueTest, givenEnqueueBlockedByUserEventWhenItIsEnqueued
     pCmdQ->enqueueKernel(mockKernel, 1, &offset, &size, &size, 1, &blockedEvent, nullptr);
     EXPECT_EQ(currentRefCount + 1, mockKernel->getRefInternalCount());
     userEvent.setStatus(CL_COMPLETE);
+    pCmdQ->isQueueBlocked();
     EXPECT_EQ(currentRefCount, mockKernel->getRefInternalCount());
 }
 
@@ -490,7 +499,7 @@ HWTEST_F(CommandQueueHwRefCountTest, givenBlockedCmdQWhenNewBlockedEnqueueReplac
 
     userEvent.setStatus(CL_COMPLETE);
     // UserEvent is set to complete and event tree is unblocked, queue has only 1 refference to itself after this operation
-    EXPECT_EQ(1, mockCmdQ->getRefInternalCount());
+    EXPECT_EQ(2, mockCmdQ->getRefInternalCount());
 
     //this call will release the queue
     releaseQueue<CommandQueue>(mockCmdQ, retVal);
@@ -530,12 +539,13 @@ HWTEST_F(CommandQueueHwRefCountTest, givenBlockedCmdQWithOutputEventAsVirtualEve
     // unblocking deletes 2 virtualEvents
     userEvent.setStatus(CL_COMPLETE);
 
-    EXPECT_EQ(2, mockCmdQ->getRefInternalCount());
+    EXPECT_EQ(3, mockCmdQ->getRefInternalCount());
 
     auto pEventOut = castToObject<Event>(eventOut);
     pEventOut->release();
     // releasing output event decrements refCount
-    EXPECT_EQ(1, mockCmdQ->getRefInternalCount());
+    EXPECT_EQ(2, mockCmdQ->getRefInternalCount());
+    mockCmdQ->isQueueBlocked();
 
     releaseQueue<CommandQueue>(mockCmdQ, retVal);
 }
@@ -575,13 +585,16 @@ HWTEST_F(CommandQueueHwRefCountTest, givenSeriesOfBlockedEnqueuesWhenEveryEventI
     userEvent->setStatus(CL_COMPLETE);
 
     userEvent->release();
-    // releasing UserEvent doesn't change the refCount
-    EXPECT_EQ(2, mockCmdQ->getRefInternalCount());
+    EXPECT_EQ(3, mockCmdQ->getRefInternalCount());
 
     auto pEventOut = castToObject<Event>(eventOut);
     pEventOut->release();
 
     // releasing output event decrements refCount
+    EXPECT_EQ(2, mockCmdQ->getRefInternalCount());
+
+    mockCmdQ->isQueueBlocked();
+
     EXPECT_EQ(1, mockCmdQ->getRefInternalCount());
 
     releaseQueue<CommandQueue>(mockCmdQ, retVal);
@@ -622,7 +635,7 @@ HWTEST_F(CommandQueueHwRefCountTest, givenSeriesOfBlockedEnqueuesWhenCmdQIsRelea
 
     userEvent->release();
     // releasing UserEvent doesn't change the queue refCount
-    EXPECT_EQ(2, mockCmdQ->getRefInternalCount());
+    EXPECT_EQ(3, mockCmdQ->getRefInternalCount());
 
     releaseQueue<CommandQueue>(mockCmdQ, retVal);
 
@@ -963,6 +976,7 @@ HWTEST_F(CommandQueueHwTest, givenWalkerSplitEnqueueNDRangeWhenBlockedThenKernel
     EXPECT_EQ(1u, mockKernel->getResidencyCalls);
 
     userEvent.setStatus(CL_COMPLETE);
+    pCmdQ->isQueueBlocked();
 }
 
 HWTEST_F(CommandQueueHwTest, givenKernelSplitEnqueueReadBufferWhenBlockedThenEnqueueSurfacesMakeResidentIsCalledOnce) {
@@ -972,7 +986,7 @@ HWTEST_F(CommandQueueHwTest, givenKernelSplitEnqueueReadBufferWhenBlockedThenEnq
     csr.timestampPacketWriteEnabled = false;
 
     BufferDefaults::context = context;
-    std::unique_ptr<Buffer> buffer(BufferHelper<>::create());
+    auto buffer = clUniquePtr(BufferHelper<>::create());
     GraphicsAllocation *bufferAllocation = buffer->getGraphicsAllocation();
     char array[3 * MemoryConstants::cacheLineSize];
     char *ptr = &array[MemoryConstants::cacheLineSize];
@@ -995,4 +1009,6 @@ HWTEST_F(CommandQueueHwTest, givenKernelSplitEnqueueReadBufferWhenBlockedThenEnq
         }
         EXPECT_EQ(expected, it->second);
     }
+
+    pCmdQ->isQueueBlocked();
 }

unit_tests/command_queue/enqueue_handler_tests.cpp

@@ -200,54 +200,11 @@ HWTEST_F(EnqueueHandlerTest, enqueueBlockedSetsVirtualEventAsCurrentCmdQVirtualE
 
     ASSERT_NE(nullptr, mockCmdQ->virtualEvent);
 
-    EXPECT_TRUE(mockCmdQ->virtualEvent->isCurrentCmdQVirtualEvent());
-
     mockCmdQ->virtualEvent->setStatus(CL_COMPLETE);
     mockCmdQ->isQueueBlocked();
     mockCmdQ->release();
 }
 
-HWTEST_F(EnqueueHandlerTest, enqueueBlockedUnsetsCurrentCmdQVirtualEventForPreviousVirtualEvent) {
-
-    UserEvent userEvent;
-    cl_event clUserEvent = &userEvent;
-
-    MockKernelWithInternals kernelInternals(*pDevice, context);
-
-    Kernel *kernel = kernelInternals.mockKernel;
-
-    MockMultiDispatchInfo multiDispatchInfo(kernel);
-
-    auto mockCmdQ = new MockCommandQueueHw<FamilyType>(context, pDevice, 0);
-
-    // put queue into initial blocked state with userEvent
-
-    bool blocking = false;
-    mockCmdQ->template enqueueHandler<CL_COMMAND_NDRANGE_KERNEL>(nullptr,
-                                                                 0,
-                                                                 blocking,
-                                                                 multiDispatchInfo,
-                                                                 1,
-                                                                 &clUserEvent,
-                                                                 nullptr);
-
-    ASSERT_NE(nullptr, mockCmdQ->virtualEvent);
-    Event *previousVirtualEvent = mockCmdQ->virtualEvent;
-
-    mockCmdQ->template enqueueHandler<CL_COMMAND_NDRANGE_KERNEL>(nullptr,
-                                                                 0,
-                                                                 blocking,
-                                                                 multiDispatchInfo,
-                                                                 0,
-                                                                 nullptr,
-                                                                 nullptr);
-
-    EXPECT_FALSE(previousVirtualEvent->isCurrentCmdQVirtualEvent());
-    EXPECT_TRUE(mockCmdQ->virtualEvent->isCurrentCmdQVirtualEvent());
-
-    mockCmdQ->release();
-}
-
 HWTEST_F(EnqueueHandlerTest, enqueueWithOutputEventRegistersEvent) {
     MockKernelWithInternals kernelInternals(*pDevice, context);
     Kernel *kernel = kernelInternals.mockKernel;

unit_tests/command_queue/enqueue_kernel_2_tests.cpp

@@ -643,6 +643,7 @@ HWTEST_P(EnqueueKernelPrintfTest, GivenKernelWithPrintfBlockedByEventWhenEventUn
 
         std::string output = testing::internal::GetCapturedStdout();
         EXPECT_STREQ("test", output.c_str());
+        pCmdQ->releaseVirtualEvent();
     }
 }