tcuThreadUtil: fix data races on m_status
authorTapani Pälli <tapani.palli@intel.com>
Tue, 27 Aug 2019 10:50:09 +0000 (13:50 +0300)
committerAlexander Galazin <Alexander.Galazin@arm.com>
Fri, 4 Oct 2019 09:57:11 +0000 (05:57 -0400)
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 <tapani.palli@intel.com>
framework/common/tcuThreadUtil.cpp
framework/common/tcuThreadUtil.hpp

index 84ac428..4ee3b33 100644 (file)
@@ -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);
        }
 }
 
index ed58c23..d6c4124 100644 (file)
@@ -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<Message>    m_messages;
+       mutable de::Mutex               m_statusLock;
        ThreadStatus                    m_status;
        std::vector<deUint8>    m_dummyData;