From f4d49e5f3ef55b0fadb361095c38256ce46920cb Mon Sep 17 00:00:00 2001 From: Diego Kluwe de Souza Date: Wed, 7 Oct 2026 17:20:47 -0300 Subject: [PATCH 1/2] Initialize and refresh SoBase auditor cache correctly --- src/misc/SoBase.cpp | 7 ++-- testsuite/SoBaseGetAuditorsTest.cpp | 36 +++++++++++++++++++ testsuite/regressions/SoBaseGetAuditors.cmake | 6 ++++ 3 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 testsuite/SoBaseGetAuditorsTest.cpp create mode 100644 testsuite/regressions/SoBaseGetAuditors.cmake diff --git a/src/misc/SoBase.cpp b/src/misc/SoBase.cpp index 0fb68738918..0e0632ad970 100644 --- a/src/misc/SoBase.cpp +++ b/src/misc/SoBase.cpp @@ -882,12 +882,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..88250d2d61e --- /dev/null +++ b/testsuite/SoBaseGetAuditorsTest.cpp @@ -0,0 +1,36 @@ +#include +#include +#include + +#include + +int +main() +{ + SoDB::init(); + SoGroup * node = new SoGroup; + node->ref(); + int first = 0, second = 0, third = 0; + node->addAuditor(&first, SoNotRec::PARENT); + node->addAuditor(&second, SoNotRec::PARENT); + node->addAuditor(&third, SoNotRec::PARENT); + + const SoAuditorList & initial = node->getAuditors(); + bool ok = initial.getLength() == 3 && + initial.find(&first, SoNotRec::PARENT) >= 0 && + initial.find(&second, SoNotRec::PARENT) >= 0 && + initial.find(&third, SoNotRec::PARENT) >= 0; + + node->removeAuditor(&second, SoNotRec::PARENT); + const SoAuditorList & refreshed = node->getAuditors(); + ok = ok && refreshed.getLength() == 2 && + refreshed.find(&first, SoNotRec::PARENT) >= 0 && + refreshed.find(&second, SoNotRec::PARENT) == -1 && + refreshed.find(&third, SoNotRec::PARENT) >= 0; + + node->removeAuditor(&first, SoNotRec::PARENT); + node->removeAuditor(&third, SoNotRec::PARENT); + node->unref(); + if (!ok) std::fprintf(stderr, "auditor cache did not match the tree\n"); + return ok ? 0 : 1; +} diff --git a/testsuite/regressions/SoBaseGetAuditors.cmake b/testsuite/regressions/SoBaseGetAuditors.cmake new file mode 100644 index 00000000000..c3c87b38fb6 --- /dev/null +++ b/testsuite/regressions/SoBaseGetAuditors.cmake @@ -0,0 +1,6 @@ +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) From 4889bb6e7aa76f255ab3573b98b65168d86fed5d Mon Sep 17 00:00:00 2001 From: Diego Kluwe de Souza Date: Thu, 8 Oct 2026 22:03:26 -0300 Subject: [PATCH 2/2] Release cached auditor lists and strengthen cache regressions --- src/misc/SoBase.cpp | 14 +- testsuite/SoBaseGetAuditorsTest.cpp | 153 +++++++++++++++--- testsuite/regressions/SoBaseGetAuditors.cmake | 4 +- 3 files changed, 142 insertions(+), 29 deletions(-) diff --git a/src/misc/SoBase.cpp b/src/misc/SoBase.cpp index 0e0632ad970..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 & diff --git a/testsuite/SoBaseGetAuditorsTest.cpp b/testsuite/SoBaseGetAuditorsTest.cpp index 88250d2d61e..13b59f0c9c0 100644 --- a/testsuite/SoBaseGetAuditorsTest.cpp +++ b/testsuite/SoBaseGetAuditorsTest.cpp @@ -3,34 +3,137 @@ #include #include +#include -int -main() +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() { - SoDB::init(); SoGroup * node = new SoGroup; node->ref(); - int first = 0, second = 0, third = 0; - node->addAuditor(&first, SoNotRec::PARENT); - node->addAuditor(&second, SoNotRec::PARENT); - node->addAuditor(&third, SoNotRec::PARENT); - - const SoAuditorList & initial = node->getAuditors(); - bool ok = initial.getLength() == 3 && - initial.find(&first, SoNotRec::PARENT) >= 0 && - initial.find(&second, SoNotRec::PARENT) >= 0 && - initial.find(&third, SoNotRec::PARENT) >= 0; - - node->removeAuditor(&second, SoNotRec::PARENT); - const SoAuditorList & refreshed = node->getAuditors(); - ok = ok && refreshed.getLength() == 2 && - refreshed.find(&first, SoNotRec::PARENT) >= 0 && - refreshed.find(&second, SoNotRec::PARENT) == -1 && - refreshed.find(&third, SoNotRec::PARENT) >= 0; - - node->removeAuditor(&first, SoNotRec::PARENT); - node->removeAuditor(&third, SoNotRec::PARENT); + 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(); - if (!ok) std::fprintf(stderr, "auditor cache did not match the tree\n"); - return ok ? 0 : 1; +} + +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 index c3c87b38fb6..1c20c752af8 100644 --- a/testsuite/regressions/SoBaseGetAuditors.cmake +++ b/testsuite/regressions/SoBaseGetAuditors.cmake @@ -3,4 +3,6 @@ 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) +add_test(NAME SoBaseGetAuditors COMMAND SoBaseGetAuditorsTest refresh) +add_test(NAME SoBaseGetAuditorsEmpty COMMAND SoBaseGetAuditorsTest empty) +add_test(NAME SoBaseGetAuditorsLifetime COMMAND SoBaseGetAuditorsTest lifetime)