From dfa0bc32104edb6682d2a8bff44d63c8f522e8ba Mon Sep 17 00:00:00 2001 From: =?utf8?q?Tapani=20P=C3=A4lli?= Date: Tue, 27 Aug 2019 13:50:09 +0300 Subject: [PATCH] tcuThreadUtil: fix data races on m_status MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Fixes data race where m_status is being read by Thread::getStatus() and simultaneously written by Thread::run(). There are multiple instances of these errors during dEQP-EGL.functional.sharing.gles2.multithread.* tests. ==14810== Possible data race during write of size 4 at 0x5E79E30 by thread #4 ==14810== Locks held: none ==14810== at 0x7214AF: tcu::ThreadUtil::Thread::run() ==14810== by 0x74F348: de::threadFunc(void*) ==14810== by 0x756B89: startThread ==14810== by 0x483F76D: mythread_wrapper (hg_intercepts.c:389) ==14810== by 0x4A195A1: start_thread (in /usr/lib64/libpthread-2.29.so) ==14810== by 0x4ED8022: clone (in /usr/lib64/libc-2.29.so) ==14810== ==14810== This conflicts with a previous read of size 4 by thread #1 ==14810== Locks held: none ==14810== at 0x48E076: tcu::ThreadUtil::Thread::getStatus() const ==14810== by 0x47E911: deqp::egl::GLES2SharingRandomTest::iterate() ==14810== by 0x45F044: deqp::egl::TestCaseWrapper::iterate(tcu::TestCase*) ==14810== by 0x6F7829: tcu::TestSessionExecutor::iterateTestCase(tcu::TestCase*) ==14810== by 0x6F6854: tcu::TestSessionExecutor::iterate() ==14810== by 0x6C84C2: tcu::App::iterate() ==14810== by 0x45A57A: main ==14810== Address 0x5e79e30 is 96 bytes inside a block of size 200 alloc'd Components: EGL VK-GL-CTS issue: 1955 Affects: dEQP-EGL.functional.sharing.gles2.multithread.* Change-Id: I9b8a069743fc2271a962e69dd0c7f18490119e97 Signed-off-by: Tapani Pälli --- framework/common/tcuThreadUtil.cpp | 12 ++++++------ framework/common/tcuThreadUtil.hpp | 4 +++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/framework/common/tcuThreadUtil.cpp b/framework/common/tcuThreadUtil.cpp index 84ac428..4ee3b33 100644 --- a/framework/common/tcuThreadUtil.cpp +++ b/framework/common/tcuThreadUtil.cpp @@ -194,7 +194,7 @@ void Thread::addOperation (Operation* operation) void Thread::run (void) { - m_status = THREADSTATUS_RUNNING; + setStatus(THREADSTATUS_RUNNING); bool initOk = false; // Reserve at least two messages for each operation @@ -207,31 +207,31 @@ void Thread::run (void) m_operations[operationNdx]->execute(*this); deinit(); - m_status = THREADSTATUS_READY; + setStatus(THREADSTATUS_READY); } catch (const tcu::NotSupportedError& e) { newMessage() << "tcu::NotSupportedError '" << e.what() << "'" << Message::End; deinit(); - m_status = (initOk ? THREADSTATUS_NOT_SUPPORTED : THREADSTATUS_INIT_FAILED); + setStatus(initOk ? THREADSTATUS_NOT_SUPPORTED : THREADSTATUS_INIT_FAILED); } catch (const tcu::Exception& e) { newMessage() << "tcu::Exception '" << e.what() << "'" << Message::End; deinit(); - m_status = (initOk ? THREADSTATUS_FAILED : THREADSTATUS_INIT_FAILED); + setStatus(initOk ? THREADSTATUS_FAILED : THREADSTATUS_INIT_FAILED); } catch (const std::exception& error) { newMessage() << "std::exception '" << error.what() << "'" << Message::End; deinit(); - m_status = (initOk ? THREADSTATUS_FAILED : THREADSTATUS_INIT_FAILED); + setStatus(initOk ? THREADSTATUS_FAILED : THREADSTATUS_INIT_FAILED); } catch (...) { newMessage() << "Unkown exception" << Message::End; deinit(); - m_status = (initOk ? THREADSTATUS_FAILED : THREADSTATUS_INIT_FAILED); + setStatus(initOk ? THREADSTATUS_FAILED : THREADSTATUS_INIT_FAILED); } } diff --git a/framework/common/tcuThreadUtil.hpp b/framework/common/tcuThreadUtil.hpp index ed58c23..d6c4124 100644 --- a/framework/common/tcuThreadUtil.hpp +++ b/framework/common/tcuThreadUtil.hpp @@ -176,7 +176,8 @@ public: deUint8* getDummyData (size_t size); //!< Return data pointer that contains at least size bytes. Valid until next call - ThreadStatus getStatus (void) const { return m_status; } + ThreadStatus getStatus (void) const { de::ScopedLock lock(m_statusLock); return m_status; } + void setStatus (ThreadStatus status) { de::ScopedLock lock(m_statusLock); m_status = status; } MessageBuilder newMessage (void) { return MessageBuilder(*this); } de::Random& getRandom (void) { return m_random; } @@ -196,6 +197,7 @@ private: mutable de::Mutex m_messageLock; std::vector m_messages; + mutable de::Mutex m_statusLock; ThreadStatus m_status; std::vector m_dummyData; -- 2.7.4