diff --git a/src/rcu.h b/src/rcu.h --- a/src/rcu.h +++ b/src/rcu.h @@ -21,6 +21,9 @@ std::atomic state; std::atomic next; + bool isCleaningUp = false; + class RCUCleanupGuard; + std::map> cleanups; // The largest revision possible means unlocked. @@ -63,7 +66,10 @@ public: RCULock() : RCULock(&RCUInfos::infos) {} - ~RCULock() { infos->readFree(); } + ~RCULock() { + infos->readFree(); + infos->runCleanups(); + } RCULock(const RCULock &) = delete; RCULock &operator=(const RCULock &) = delete; diff --git a/src/rcu.cpp b/src/rcu.cpp --- a/src/rcu.cpp +++ b/src/rcu.cpp @@ -194,21 +194,40 @@ })); } +class RCUInfos::RCUCleanupGuard { + RCUInfos *infos; + +public: + explicit RCUCleanupGuard(RCUInfos *infosIn) : infos(infosIn) { + infos->isCleaningUp = true; + } + + ~RCUCleanupGuard() { infos->isCleaningUp = false; } +}; + void RCUInfos::runCleanups() { + if (isCleaningUp || cleanups.empty()) { + // We don't want to run cleanups within cleanups. + return; + } + + RCUCleanupGuard guard(this); + // By the time we run a set of cleanups, we may have more cleanups // available so we loop until there is nothing available for cleanup. - while (true) { - if (cleanups.empty()) { - // There is nothing to cleanup. + while (!cleanups.empty()) { + auto it = cleanups.begin(); + uint64_t syncedTo = hasSyncedTo(it->first); + if (it->first > syncedTo) { + // We have nothing more ready to be cleaned up. return; } - auto it = cleanups.begin(); - uint64_t syncedTo = hasSyncedTo(it->first); while (it != cleanups.end() && it->first <= syncedTo) { // Run the cleanup and remove it from the map. - it->second(); + auto fun = std::move(it->second); cleanups.erase(it++); + fun(); } } } diff --git a/src/test/rcu_tests.cpp b/src/test/rcu_tests.cpp --- a/src/test/rcu_tests.cpp +++ b/src/test/rcu_tests.cpp @@ -136,7 +136,7 @@ BOOST_CHECK(true); } -BOOST_AUTO_TEST_CASE(cleanup_test) { +BOOST_AUTO_TEST_CASE(cleanup_simple) { RCULock::synchronize(); BOOST_CHECK(RCUTest::getCleanups().empty()); @@ -145,16 +145,19 @@ BOOST_CHECK(!isClean1); BOOST_CHECK_EQUAL(RCUTest::getCleanups().size(), 1); - BOOST_CHECK_EQUAL(RCUTest::getRevision(), - RCUTest::getCleanups().begin()->first); + + auto revision = RCUTest::getCleanups().begin()->first; + BOOST_CHECK_EQUAL(RCUTest::getRevision(), revision); // Synchronize runs the cleanups. RCULock::synchronize(); BOOST_CHECK(RCUTest::getCleanups().empty()); BOOST_CHECK(isClean1); +} +BOOST_AUTO_TEST_CASE(cleanup_multiple) { // Check multiple callbacks. - isClean1 = false; + bool isClean1 = false; bool isClean2 = false; bool isClean3 = false; RCULock::registerCleanup([&] { isClean1 = true; }); @@ -167,11 +170,13 @@ BOOST_CHECK(isClean1); BOOST_CHECK(isClean2); BOOST_CHECK(isClean3); +} +BOOST_AUTO_TEST_CASE(cleanup_test_nested) { // Check callbacks adding each others. - isClean1 = false; - isClean2 = false; - isClean3 = false; + bool isClean1 = false; + bool isClean2 = false; + bool isClean3 = false; RCULock::registerCleanup([&] { isClean1 = true; @@ -189,6 +194,33 @@ BOOST_CHECK(isClean3); } +BOOST_AUTO_TEST_CASE(cleanup_on_unlock) { + // Check callbacks adding each others. + bool isClean1 = false; + bool isClean2 = false; + bool isClean3 = false; + + RCULock::registerCleanup([&] { + isClean1 = true; + RCULock::registerCleanup([&] { + isClean2 = true; + RCULock::registerCleanup([&] { isClean3 = true; }); + }); + }); + + BOOST_CHECK_EQUAL(RCUTest::getCleanups().size(), 1); + + { + // There is no contention, so this will cleanup. + RCULock lock; + } + + BOOST_CHECK(RCUTest::getCleanups().empty()); + BOOST_CHECK(isClean1); + BOOST_CHECK(isClean2); + BOOST_CHECK(isClean3); +} + class RCURefTestItem { IMPLEMENT_RCU_REFCOUNT(uint32_t); const std::function cleanupfun;