From 68ef903a7c60e78cb74239ff781a90040a2e9082 Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Wed, 26 Jul 2017 22:45:55 +0300 Subject: [PATCH 1/2] core(tls): don't use tlsSlots without synchronization --- modules/core/src/system.cpp | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/modules/core/src/system.cpp b/modules/core/src/system.cpp index a45853c403..629c3fb637 100644 --- a/modules/core/src/system.cpp +++ b/modules/core/src/system.cpp @@ -1306,7 +1306,8 @@ struct ThreadData class TlsStorage { public: - TlsStorage() + TlsStorage() : + tlsSlotsSize(0) { tlsSlots.reserve(32); threads.reserve(32); @@ -1351,9 +1352,10 @@ public: size_t reserveSlot() { AutoLock guard(mtxGlobalAccess); + CV_Assert(tlsSlotsSize == tlsSlots.size()); // Find unused slots - for(size_t slot = 0; slot < tlsSlots.size(); slot++) + for(size_t slot = 0; slot < tlsSlotsSize; slot++) { if(!tlsSlots[slot]) { @@ -1363,15 +1365,16 @@ public: } // Create new slot - tlsSlots.push_back(1); - return (tlsSlots.size()-1); + tlsSlots.push_back(1); tlsSlotsSize++; + return tlsSlotsSize - 1; } // Release TLS storage index and pass associated data to caller void releaseSlot(size_t slotIdx, std::vector &dataVec, bool keepSlot = false) { AutoLock guard(mtxGlobalAccess); - CV_Assert(tlsSlots.size() > slotIdx); + CV_Assert(tlsSlotsSize == tlsSlots.size()); + CV_Assert(tlsSlotsSize > slotIdx); for(size_t i = 0; i < threads.size(); i++) { @@ -1393,7 +1396,7 @@ public: // Get data by TLS storage index void* getData(size_t slotIdx) const { - CV_Assert(tlsSlots.size() > slotIdx); + CV_Assert(tlsSlotsSize > slotIdx); ThreadData* threadData = (ThreadData*)tls.GetData(); if(threadData && threadData->slots.size() > slotIdx) @@ -1406,7 +1409,8 @@ public: void gather(size_t slotIdx, std::vector &dataVec) { AutoLock guard(mtxGlobalAccess); - CV_Assert(tlsSlots.size() > slotIdx); + CV_Assert(tlsSlotsSize == tlsSlots.size()); + CV_Assert(tlsSlotsSize > slotIdx); for(size_t i = 0; i < threads.size(); i++) { @@ -1422,7 +1426,7 @@ public: // Set data to storage index void setData(size_t slotIdx, void* pData) { - CV_Assert(tlsSlots.size() > slotIdx && pData != NULL); + CV_Assert(tlsSlotsSize > slotIdx); ThreadData* threadData = (ThreadData*)tls.GetData(); if(!threadData) @@ -1438,9 +1442,8 @@ public: if(slotIdx >= threadData->slots.size()) { - AutoLock guard(mtxGlobalAccess); - while(slotIdx >= threadData->slots.size()) - threadData->slots.push_back(NULL); + AutoLock guard(mtxGlobalAccess); // keep synchronization with gather() calls + threadData->slots.resize(slotIdx + 1, NULL); } threadData->slots[slotIdx] = pData; } @@ -1449,6 +1452,8 @@ private: TlsAbstraction tls; // TLS abstraction layer instance Mutex mtxGlobalAccess; // Shared objects operation guard + size_t tlsSlotsSize; // equal to tlsSlots.size() in synchronized sections + // without synchronization this counter doesn't desrease - it is used for slotIdx sanity checks std::vector tlsSlots; // TLS keys state std::vector threads; // Array for all allocated data. Thread data pointers are placed here to allow data cleanup }; From d35422b5234761deef85e46ee84f109fb11010e9 Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Thu, 27 Jul 2017 17:31:51 +0300 Subject: [PATCH 2/2] core(tls): hide assertions from Thread Sanitizer --- modules/core/include/opencv2/core/cvdef.h | 11 +++++++++++ modules/core/src/system.cpp | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/modules/core/include/opencv2/core/cvdef.h b/modules/core/include/opencv2/core/cvdef.h index 2ac0e17055..70bbf93104 100644 --- a/modules/core/include/opencv2/core/cvdef.h +++ b/modules/core/include/opencv2/core/cvdef.h @@ -327,6 +327,17 @@ Cv64suf; # endif #endif +/****************************************************************************************\ +* Thread sanitizer * +\****************************************************************************************/ +#ifndef CV_THREAD_SANITIZER +# if defined(__has_feature) +# if __has_feature(thread_sanitizer) +# define CV_THREAD_SANITIZER +# endif +# endif +#endif + /****************************************************************************************\ * exchange-add operation for atomic operations on reference counters * \****************************************************************************************/ diff --git a/modules/core/src/system.cpp b/modules/core/src/system.cpp index 629c3fb637..7ea0dd6c5d 100644 --- a/modules/core/src/system.cpp +++ b/modules/core/src/system.cpp @@ -1396,7 +1396,9 @@ public: // Get data by TLS storage index void* getData(size_t slotIdx) const { +#ifndef CV_THREAD_SANITIZER CV_Assert(tlsSlotsSize > slotIdx); +#endif ThreadData* threadData = (ThreadData*)tls.GetData(); if(threadData && threadData->slots.size() > slotIdx) @@ -1426,7 +1428,9 @@ public: // Set data to storage index void setData(size_t slotIdx, void* pData) { +#ifndef CV_THREAD_SANITIZER CV_Assert(tlsSlotsSize > slotIdx); +#endif ThreadData* threadData = (ThreadData*)tls.GetData(); if(!threadData)