diff --git a/runtime/memory_manager/deferred_deleter.cpp b/runtime/memory_manager/deferred_deleter.cpp index a0542fd5c0..4611658ddc 100644 --- a/runtime/memory_manager/deferred_deleter.cpp +++ b/runtime/memory_manager/deferred_deleter.cpp @@ -1,5 +1,5 @@ /* -* Copyright (c) 2017, Intel Corporation +* Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -26,30 +26,41 @@ namespace OCLRT { DeferredDeleter::DeferredDeleter() { doWorkInBackground = false; - threadLoaded = false; elementsToRelease = 0; } void DeferredDeleter::stop() { - if (worker) { - while (!threadLoaded) - ; - threadLoaded = false; + // Called with threadMutex acquired + if (worker != nullptr) { + // Working thread was created so we can safely stop it + std::unique_lock lock(queueMutex); + // Make sure that working thread really started + while (!doWorkInBackground) { + lock.unlock(); + lock.lock(); + } + // Signal working thread to finish its job doWorkInBackground = false; - queueMutex.lock(); - queueMutex.unlock(); + lock.unlock(); condition.notify_one(); + // Wait for the working job to exit worker->join(); + // Delete working thread delete worker; worker = nullptr; } drain(false); } -DeferredDeleter::~DeferredDeleter() { +void DeferredDeleter::safeStop() { + std::lock_guard lock(threadMutex); stop(); } +DeferredDeleter::~DeferredDeleter() { + safeStop(); +} + void DeferredDeleter::deferDeletion(DeferrableDeletion *deletion) { std::unique_lock lock(queueMutex); elementsToRelease++; @@ -73,8 +84,9 @@ void DeferredDeleter::removeClient() { } void DeferredDeleter::ensureThread() { - if (worker) + if (worker != nullptr) { return; + } worker = new std::thread(run, this); } @@ -88,15 +100,18 @@ bool DeferredDeleter::shouldStop() { void DeferredDeleter::run(DeferredDeleter *self) { std::unique_lock lock(self->queueMutex); + // Mark that working thread really started self->doWorkInBackground = true; - self->threadLoaded = true; do { if (self->queue.peekIsEmpty()) { + // Wait for signal that some items are ready to be deleted self->condition.wait(lock); } lock.unlock(); + // Delete items placed into deferred delete queue self->clearQueue(); lock.lock(); + // Check whether working thread should be stopped } while (!self->shouldStop()); lock.unlock(); } diff --git a/runtime/memory_manager/deferred_deleter.h b/runtime/memory_manager/deferred_deleter.h index cb37c1159b..52f4afc40e 100644 --- a/runtime/memory_manager/deferred_deleter.h +++ b/runtime/memory_manager/deferred_deleter.h @@ -1,5 +1,5 @@ /* -* Copyright (c) 2017, Intel Corporation +* Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -48,6 +48,7 @@ class DeferredDeleter { protected: void stop(); + void safeStop(); void ensureThread(); MOCKABLE_VIRTUAL void clearQueue(); MOCKABLE_VIRTUAL bool areElementsReleased(); @@ -56,7 +57,6 @@ class DeferredDeleter { static void run(DeferredDeleter *self); std::atomic doWorkInBackground; - std::atomic threadLoaded; std::atomic elementsToRelease; std::thread *worker = nullptr; int32_t numClients = 0; diff --git a/unit_tests/memory_manager/deferred_deleter_mt_tests.cpp b/unit_tests/memory_manager/deferred_deleter_mt_tests.cpp index 9c3fb37072..cbd404f663 100644 --- a/unit_tests/memory_manager/deferred_deleter_mt_tests.cpp +++ b/unit_tests/memory_manager/deferred_deleter_mt_tests.cpp @@ -1,5 +1,5 @@ /* -* Copyright (c) 2017, Intel Corporation +* Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -49,7 +49,7 @@ struct DeferredDeleterTest : public ::testing::Test { } void waitForAsyncThread() { - while (!deleter->isThreadLoaded()) { + while (!deleter->isWorking()) { std::this_thread::yield(); } } @@ -63,7 +63,6 @@ TEST_F(DeferredDeleterTest, initialValues) { EXPECT_EQ(0, deleter->getClientsNum()); EXPECT_FALSE(deleter->isWorking()); EXPECT_FALSE(deleter->isThreadRunning()); - EXPECT_FALSE(deleter->isThreadLoaded()); EXPECT_EQ(0, deleter->drainCalled); EXPECT_EQ(0, deleter->clearCalled); EXPECT_EQ(0, deleter->areElementsReleasedCalled); diff --git a/unit_tests/mocks/mock_deferred_deleter.cpp b/unit_tests/mocks/mock_deferred_deleter.cpp index dd980c40c2..dac69ebad8 100644 --- a/unit_tests/mocks/mock_deferred_deleter.cpp +++ b/unit_tests/mocks/mock_deferred_deleter.cpp @@ -1,5 +1,5 @@ /* -* Copyright (c) 2017, Intel Corporation +* Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -84,10 +84,6 @@ bool MockDeferredDeleter::isWorking() { return doWorkInBackground; } -bool MockDeferredDeleter::isThreadLoaded() { - return threadLoaded; -} - bool MockDeferredDeleter::isThreadRunning() { return worker != nullptr; } diff --git a/unit_tests/mocks/mock_deferred_deleter.h b/unit_tests/mocks/mock_deferred_deleter.h index f19745acaa..bf561e3bcf 100644 --- a/unit_tests/mocks/mock_deferred_deleter.h +++ b/unit_tests/mocks/mock_deferred_deleter.h @@ -1,5 +1,5 @@ /* -* Copyright (c) 2017, Intel Corporation +* Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -50,8 +50,6 @@ class MockDeferredDeleter : public DeferredDeleter { bool isWorking(); - bool isThreadLoaded(); - bool isThreadRunning(); bool isQueueEmpty(); diff --git a/unit_tests/mt_tests/memory_manager/deferred_deleter_clear_queue_mt_tests.cpp b/unit_tests/mt_tests/memory_manager/deferred_deleter_clear_queue_mt_tests.cpp index f7f9b14b3e..d197e4f577 100644 --- a/unit_tests/mt_tests/memory_manager/deferred_deleter_clear_queue_mt_tests.cpp +++ b/unit_tests/mt_tests/memory_manager/deferred_deleter_clear_queue_mt_tests.cpp @@ -1,5 +1,5 @@ /* -* Copyright (c) 2017, Intel Corporation +* Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -82,3 +82,69 @@ int paramsForClearQueueTest[] = {1, 10, 20, 50, 100}; INSTANTIATE_TEST_CASE_P(DeferredDeleterMtTests, ClearQueueTest, ::testing::ValuesIn(paramsForClearQueueTest)); + +class MyDeferredDeleter : public DeferredDeleter { + public: + bool isQueueEmpty() { + std::lock_guard lock(queueMutex); + return queue.peekIsEmpty(); + } + int getElementsToRelease() { + return elementsToRelease; + } + bool isWorking() { + return doWorkInBackground; + } + bool isThreadRunning() { + return worker != nullptr; + } + int getClientsNum() { + return numClients; + } + void forceSafeStop() { + safeStop(); + } +}; + +struct DeferredDeleterMtTest : public ::testing::Test { + + void SetUp() override { + deleter.reset(new MyDeferredDeleter()); + } + + void TearDown() override { + EXPECT_TRUE(deleter->isQueueEmpty()); + EXPECT_EQ(0, deleter->getElementsToRelease()); + } + + void waitForAsyncThread() { + while (!deleter->isWorking()) { + std::this_thread::yield(); + } + } + std::unique_ptr deleter; +}; + +TEST_F(DeferredDeleterMtTest, asyncThreadsStopDeferredDeleter) { + deleter->addClient(); + + waitForAsyncThread(); + EXPECT_TRUE(deleter->isThreadRunning()); + EXPECT_TRUE(deleter->isWorking()); + + // Start worker thread + std::thread t([&]() { + deleter->forceSafeStop(); + EXPECT_FALSE(deleter->isThreadRunning()); + EXPECT_FALSE(deleter->isWorking()); + }); + + deleter->forceSafeStop(); + EXPECT_FALSE(deleter->isThreadRunning()); + EXPECT_FALSE(deleter->isWorking()); + + t.join(); + + deleter->removeClient(); + EXPECT_EQ(0, deleter->getClientsNum()); +}