From e61d2724e68a21f2855b9dabf917f9e48d42f5e4 Mon Sep 17 00:00:00 2001 From: Stephen Seo Date: Thu, 20 Jan 2022 14:31:23 +0900 Subject: [PATCH] Fix possible data-race with isAlive() calls Removed isAlive() calls from functions passed to ThreadPool, and instead use an unordered_set prior to using ThreadPool to "cache" which entities are not alive. This is so that if a function passed to ThreadPool modifies the ECManager (e.g. creating a new entity), then there will be no data race from also calling isAlive(). (Note that this does not protect against "deleting" entities from the ECManager during use of ThreadPool. It is expected for the caller to do the "deleting" after use of stored/called functions is finished.) --- src/EC/Manager.hpp | 85 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 69 insertions(+), 16 deletions(-) diff --git a/src/EC/Manager.hpp b/src/EC/Manager.hpp index 89e11c3..716c8a9 100644 --- a/src/EC/Manager.hpp +++ b/src/EC/Manager.hpp @@ -105,6 +105,7 @@ namespace EC EntitiesType *entities; const BitsetType *signature; void *userData; + std::unordered_set dead; }; /// Temporary struct used internally by ThreadPool template @@ -115,6 +116,7 @@ namespace EC BitsetType *signature; void *userData; Function *fn; + std::unordered_set dead; }; /// Temporary struct used internally by ThreadPool struct TPFnDataStructTwo { @@ -123,6 +125,7 @@ namespace EC EntitiesType *entities; void *userData; const std::vector *matching; + std::unordered_set dead; }; /// Temporary struct used internally by ThreadPool struct TPFnDataStructThree { @@ -132,6 +135,7 @@ namespace EC const std::vector *bitsets; EntitiesType *entities; std::mutex *mutex; + std::unordered_set dead; }; /// Temporary struct used internally by ThreadPool struct TPFnDataStructFour { @@ -141,6 +145,7 @@ namespace EC multiMatchingEntities; BitsetType *signatures; std::mutex *mutex; + std::unordered_set dead; }; /// Temporary struct used internally by ThreadPool struct TPFnDataStructFive { @@ -150,6 +155,7 @@ namespace EC void *userData; std::vector >* multiMatchingEntities; + std::unordered_set dead; }; /// Temporary struct used internally by ThreadPool struct TPFnDataStructSix { @@ -159,6 +165,7 @@ namespace EC multiMatchingEntities; BitsetType *bitsets; std::mutex *mutex; + std::unordered_set dead; }; /// Temporary struct used internally by ThreadPool template @@ -168,6 +175,7 @@ namespace EC EntitiesType *entities; Iterable *iterable; void *userData; + std::unordered_set dead; }; // end section for "temporary" structures }}} @@ -750,12 +758,17 @@ namespace EC fnDataAr[i].entities = &entities; fnDataAr[i].signature = &signatureBitset; fnDataAr[i].userData = userData; + for(std::size_t j = begin; j < end; ++j) { + if(!isAlive(j)) { + fnDataAr[i].dead.insert(j); + } + } threadPool->queueFn([&function] (void *ud) { auto *data = static_cast(ud); for(std::size_t i = data->range[0]; i < data->range[1]; ++i) { - if(!data->manager->isAlive(i)) { + if(data->dead.find(i) != data->dead.end()) { continue; } @@ -868,11 +881,16 @@ namespace EC fnDataAr[i].signature = &signatureBitset; fnDataAr[i].userData = userData; fnDataAr[i].fn = function; + for(std::size_t j = begin; j < end; ++j) { + if(!isAlive(j)) { + fnDataAr[i].dead.insert(j); + } + } threadPool->queueFn([] (void *ud) { auto *data = static_cast*>(ud); for(std::size_t i = data->range[0]; i < data->range[1]; ++i) { - if(!data->manager->isAlive(i)) { + if(data->dead.find(i) != data->dead.end()) { continue; } @@ -1013,13 +1031,17 @@ namespace EC fnDataAr[i].entities = &entities; fnDataAr[i].userData = userData; fnDataAr[i].matching = &matching; + for(std::size_t j = begin; j < end; ++j) { + if(!isAlive(matching.at(j))) { + fnDataAr[i].dead.insert(j); + } + } threadPool->queueFn([&function, helper] (void* ud) { auto *data = static_cast(ud); for(std::size_t i = data->range[0]; i < data->range[1]; ++i) { - if(data->manager->isAlive( - data->matching->at(i))) { + if(data->dead.find(i) == data->dead.end()) { helper.callInstancePtr( data->matching->at(i), *data->manager, @@ -1083,11 +1105,16 @@ namespace EC fnDataAr[i].bitsets = &bitsets; fnDataAr[i].entities = &entities; fnDataAr[i].mutex = &mutex; + for(std::size_t j = begin; j < end; ++j) { + if(!isAlive(j)) { + fnDataAr[i].dead.insert(j); + } + } threadPool->queueFn([] (void *ud) { auto *data = static_cast(ud); for(std::size_t i = data->range[0]; i < data->range[1]; ++i) { - if(!data->manager->isAlive(i)) { + if(data->dead.find(i) != data->dead.end()) { continue; } for(std::size_t j = 0; j < data->bitsets->size(); @@ -1455,12 +1482,17 @@ namespace EC fnDataAr[i].multiMatchingEntities = &multiMatchingEntities; fnDataAr[i].signatures = signatureBitsets; fnDataAr[i].mutex = &mutex; + for(std::size_t j = begin; j < end; ++j) { + if(!isAlive(j)) { + fnDataAr[i].dead.insert(j); + } + } threadPool->queueFn([] (void *ud) { auto *data = static_cast(ud); for(std::size_t i = data->range[0]; i < data->range[1]; ++i) { - if(!data->manager->isAlive(i)) { + if(data->dead.find(i) != data->dead.end()) { continue; } for(std::size_t j = 0; j < SigList::size; ++j) { @@ -1522,13 +1554,16 @@ namespace EC fnDataAr[i].userData = userData; fnDataAr[i].multiMatchingEntities = &multiMatchingEntities; + for(std::size_t j = begin; j < end; ++j) { + if(!isAlive(multiMatchingEntities.at(index).at(j))) { + fnDataAr[i].dead.insert(j); + } + } threadPool->queueFn([&func] (void *ud) { auto *data = static_cast(ud); for(std::size_t i = data->range[0]; i < data->range[1]; ++i) { - if(data->manager->isAlive( - data->multiMatchingEntities - ->at(data->index).at(i))) { + if(data->dead.find(i) == data->dead.end()) { Helper::call( data->multiMatchingEntities ->at(data->index).at(i), @@ -1655,12 +1690,17 @@ namespace EC fnDataAr[i].multiMatchingEntities = &multiMatchingEntities; fnDataAr[i].bitsets = signatureBitsets; fnDataAr[i].mutex = &mutex; + for(std::size_t j = begin; j < end; ++j) { + if(!isAlive(j)) { + fnDataAr[i].dead.insert(j); + } + } threadPool->queueFn([] (void *ud) { auto *data = static_cast(ud); for(std::size_t i = data->range[0]; i < data->range[1]; ++i) { - if(!data->manager->isAlive(i)) { + if(data->dead.find(i) != data->dead.end()) { continue; } for(std::size_t j = 0; j < SigList::size; ++j) { @@ -1727,13 +1767,16 @@ namespace EC fnDataAr[i].userData = userData; fnDataAr[i].multiMatchingEntities = &multiMatchingEntities; + for(std::size_t j = begin; j < end; ++j) { + if(!isAlive(multiMatchingEntities.at(index).at(j))) { + fnDataAr[i].dead.insert(j); + } + } threadPool->queueFn([&func] (void *ud) { auto *data = static_cast(ud); for(std::size_t i = data->range[0]; i < data->range[1]; ++i) { - if(data->manager->isAlive( - data->multiMatchingEntities - ->at(data->index).at(i))) { + if(data->dead.find(i) == data->dead.end()) { Helper::callPtr( data->multiMatchingEntities ->at(data->index).at(i), @@ -1807,13 +1850,18 @@ namespace EC fnDataAr[i].entities = &entities; fnDataAr[i].signature = &signatureBitset; fnDataAr[i].userData = userData; + for(std::size_t j = begin; j < end; ++j) { + if(!isAlive(j)) { + fnDataAr[i].dead.insert(j); + } + } threadPool->queueFn([&fn] (void *ud) { auto *data = static_cast(ud); for(std::size_t i = data->range[0]; i < data->range[1]; ++i) { - if(!data->manager->isAlive(i)) { + if(data->dead.find(i) != data->dead.end()) { continue; - } else if((*data->signature + } else if((*data->signature & std::get( data->entities->at(i))) == *data->signature) { @@ -1889,12 +1937,17 @@ namespace EC fnDataAr[i].entities = &entities; fnDataAr[i].iterable = &iterable; fnDataAr[i].userData = userData; + for(std::size_t j = begin; j < end; ++j) { + if(!isAlive(j)) { + fnDataAr[i].dead.insert(j); + } + } threadPool->queueFn([&fn] (void *ud) { auto *data = static_cast*>(ud); bool isValid; for(std::size_t i = data->range[0]; i < data->range[1]; ++i) { - if(!data->manager->isAlive(i)) { + if(data->dead.find(i) != data->dead.end()) { continue; } isValid = true;