diff --git a/src/misc/SoBase.cpp b/src/misc/SoBase.cpp index 0fb68738918..1e2072b2e31 100644 --- a/src/misc/SoBase.cpp +++ b/src/misc/SoBase.cpp @@ -240,13 +240,17 @@ SoBase::~SoBase() // used to check that we are still alive. this->objdata.alive = (~ALIVE_PATTERN) & 0xf; + CC_MUTEX_LOCK(SoBase::PImpl::auditor_mutex); if (SoBase::PImpl::auditordict) { - //SoAuditorList * l; - if (SoBase::PImpl::auditordict->find(this)!=SoBase::PImpl::auditordict->const_end()) { + SbHash::const_iterator iter = + SoBase::PImpl::auditordict->find(this); + if (iter != SoBase::PImpl::auditordict->const_end()) { + SoAuditorList * l = iter->obj; SoBase::PImpl::auditordict->erase(this); - //delete l; + delete l; } } + CC_MUTEX_UNLOCK(SoBase::PImpl::auditor_mutex); cc_rbptree_clean(&this->auditortree); #if COIN_DEBUG @@ -863,6 +867,10 @@ sobase_audlist_add(void * pointer, void * type, void * closure) /*! Returns list of objects auditing this object. + The returned list is owned by Coin and is refreshed by subsequent calls + for this object. Do not use it after this object is destroyed or after + SoDB::finish(). + \sa addAuditor(), removeAuditor() */ const SoAuditorList & @@ -882,12 +890,13 @@ SoBase::getAuditors(void) const if (iter!=SoBase::PImpl::auditordict->const_end()) { l = iter->obj; // empty list before copying in new values - for (int i = 0; i < l->getLength(); i++) { - l->remove(i); + for (int i = l->getLength(); i > 0; --i) { + l->remove(i - 1); } } else { - (*SoBase::PImpl::auditordict)[this] = new SoAuditorList; + l = new SoAuditorList; + (*SoBase::PImpl::auditordict)[this] = l; } cc_rbptree_traverse(&this->auditortree, (cc_rbptree_traversecb*)sobase_audlist_add, l); diff --git a/testsuite/SoBaseGetAuditorsTest.cpp b/testsuite/SoBaseGetAuditorsTest.cpp new file mode 100644 index 00000000000..13b59f0c9c0 --- /dev/null +++ b/testsuite/SoBaseGetAuditorsTest.cpp @@ -0,0 +1,139 @@ +#include +#include +#include + +#include +#include + +namespace { +struct Auditor { + void * pointer; + SoNotRec::Type type; +}; + +int failures = 0; + +void check(const bool condition, const char * message) +{ + if (!condition) { + std::fprintf(stderr, "FAIL: %s\n", message); + ++failures; + } +} + +bool matches(const SoAuditorList & actual, const Auditor * expected, + const int length) +{ + if (actual.getLength() != length) return false; + for (int i = 0; i < length; ++i) { + int actualcount = 0, expectedcount = 0; + for (int j = 0; j < length; ++j) { + if (actual.getObject(j) == expected[i].pointer && + actual.getType(j) == expected[i].type) ++actualcount; + if (expected[j].pointer == expected[i].pointer && + expected[j].type == expected[i].type) ++expectedcount; + } + if (actualcount != expectedcount) return false; + } + return true; +} + +void checkEmpty() +{ + SoGroup * node = new SoGroup; + node->ref(); + const SoAuditorList & cached = node->getAuditors(); + check(cached.getLength() == 0, "initial empty cache is not empty"); + check(&node->getAuditors() == &cached && cached.getLength() == 0, + "repeated empty query changed the cache"); + + int token = 0; + const Auditor expected[] = { { &token, SoNotRec::PARENT } }; + node->addAuditor(&token, SoNotRec::PARENT); + check(matches(node->getAuditors(), expected, 1), + "empty cache did not acquire the added auditor"); + node->removeAuditor(&token, SoNotRec::PARENT); + check(node->getAuditors().getLength() == 0, + "cache did not become empty after removing the last auditor"); + node->unref(); +} + +void checkRefresh() +{ + SoGroup * node = new SoGroup; + node->ref(); + int tokens[4] = {}; + const Auditor initial[] = { + { &tokens[0], SoNotRec::PARENT }, + { &tokens[1], SoNotRec::FIELD }, + { &tokens[2], SoNotRec::PARENT } + }; + for (int i = 0; i < 3; ++i) node->addAuditor(initial[i].pointer, initial[i].type); + const SoAuditorList & cached = node->getAuditors(); + check(matches(cached, initial, 3), "first populated query lost auditor pairs"); + for (int i = 0; i < 3; ++i) { + const SoAuditorList & refreshed = node->getAuditors(); + check(&refreshed == &cached && matches(refreshed, initial, 3), + "unchanged tree accumulated stale cache entries"); + } + + node->removeAuditor(initial[1].pointer, initial[1].type); + const Auditor remaining[] = { initial[0], initial[2] }; + check(matches(node->getAuditors(), remaining, 2), + "removed auditor remained in the refreshed cache"); + node->addAuditor(&tokens[3], SoNotRec::FIELD); + node->addAuditor(initial[0].pointer, initial[0].type); + const Auditor expanded[] = { + initial[0], initial[2], { &tokens[3], SoNotRec::FIELD }, initial[0] + }; + check(matches(node->getAuditors(), expanded, 4), + "cache lost auditor types or duplicate registrations"); + node->removeAuditor(initial[0].pointer, initial[0].type); + node->removeAuditor(initial[0].pointer, initial[0].type); + node->removeAuditor(initial[2].pointer, initial[2].type); + node->removeAuditor(&tokens[3], SoNotRec::FIELD); + check(node->getAuditors().getLength() == 0, + "clearing the tree left cached auditor pairs"); + check(node->getAuditors().getLength() == 0, + "repeated query of cleared tree changed the cache"); + node->unref(); +} + +void checkLifetime() +{ + SoGroup * other = new SoGroup; + other->ref(); + const SoAuditorList & othercache = other->getAuditors(); + for (int i = 0; i < 8; ++i) { + SoGroup * node = new SoGroup; + node->ref(); + int token = 0; + const Auditor expected[] = { { &token, SoNotRec::PARENT } }; + node->addAuditor(&token, SoNotRec::PARENT); + check(matches(node->getAuditors(), expected, 1), + "new object inherited another object's cache"); + node->removeAuditor(&token, SoNotRec::PARENT); + node->unref(); + check(&other->getAuditors() == &othercache && othercache.getLength() == 0, + "destroying an object changed another object's cache"); + } + other->unref(); +} + +int runTest(const char * mode) +{ + if (std::strcmp(mode, "empty") == 0) checkEmpty(); + else if (std::strcmp(mode, "refresh") == 0) checkRefresh(); + else if (std::strcmp(mode, "lifetime") == 0) checkLifetime(); + else return 2; + return failures == 0 ? 0 : 1; +} +} // namespace + +int main(int argc, char ** argv) +{ + SoDB::init(); + const int result = runTest(argc > 1 ? argv[1] : "refresh"); + SoDB::finish(); + return result; +} diff --git a/testsuite/regressions/SoBaseGetAuditors.cmake b/testsuite/regressions/SoBaseGetAuditors.cmake new file mode 100644 index 00000000000..1c20c752af8 --- /dev/null +++ b/testsuite/regressions/SoBaseGetAuditors.cmake @@ -0,0 +1,8 @@ +add_executable(SoBaseGetAuditorsTest SoBaseGetAuditorsTest.cpp) +target_link_libraries(SoBaseGetAuditorsTest Coin ${COIN_TARGET_LINK_LIBRARIES}) +target_include_directories(SoBaseGetAuditorsTest PRIVATE + ${PROJECT_SOURCE_DIR}/include ${PROJECT_BINARY_DIR}/include + ${COIN_TARGET_INCLUDE_DIRECTORIES}) +add_test(NAME SoBaseGetAuditors COMMAND SoBaseGetAuditorsTest refresh) +add_test(NAME SoBaseGetAuditorsEmpty COMMAND SoBaseGetAuditorsTest empty) +add_test(NAME SoBaseGetAuditorsLifetime COMMAND SoBaseGetAuditorsTest lifetime)