WIP: dns: Fix Boost serialization singleton assertion error during shutdown

TODO: This needs very careful review, it's written by AI.

The test_dns test was crashing with:
  Assertion '! is_destroyed()' failed in Boost serialization singleton

Root cause: The static const string DnsCache::DoNotUseCacheFile was being
destroyed during program shutdown before the global fixture's destructor ran.
This caused string comparisons to compare with corrupted memory, leading to
incorrect branch execution and archive operations during program exit.

Fixes:
- Add UseCacheFile boolean flag set in constructor, used instead of comparing
  CacheFile against DoNotUseCacheFile to avoid string comparison during shutdown
- Set InShutdown flag at the very start of destructor before any other operations
- Skip scheduling save timer for caches that don't use a file
- Use ForceSave flag to allow saving from destructor even when InShutdown is true
- Add proper shutdown checks in remove_old_entries() to skip unnecessary work
- Fix test fixture destruction order to reset Cache and Master before IoService

diff --git a/src/dns/dnscache.cpp b/src/dns/dnscache.cpp
index 0e9a270..a77ca35 100644 (file)
--- a/src/dns/dnscache.cpp
+++ b/src/dns/dnscache.cpp
@@ -68,26 +68,60 @@ DnsCache::DnsCache(const IoServiceItem &io_serv,
     , CacheFile( cache_file )
     , HasChanged( false )
     , MinTimeBetweenResolves( min_time_between_resolves )
+    , InShutdown( false )
+    , ForceSave( false )
+    , UseCacheFile( cache_file != DoNotUseCacheFile )
 {
     // load cache from file
     load_from_cachefile();
 
-    // schedule next save
-    (void) SaveTimer.expires_from_now( seconds( Config::SAVE_TIMER_SECONDS ) );
-    SaveTimer.async_wait( bind( &DnsCache::schedule_save, this,
-                                boost::asio::placeholders::error ) );
+    // Only schedule the timer if we're actually using a cache file.
+    // For caches that don't use a file (like the global fixture in tests),
+    // there's no need to schedule saves and it avoids issues during shutdown.
+    if (UseCacheFile)
+    {
+        (void) SaveTimer.expires_from_now( seconds( Config::SAVE_TIMER_SECONDS ) );
+        try
+        {
+            SaveTimer.async_wait( bind( &DnsCache::schedule_save, shared_from_this(),
+                                        boost::asio::placeholders::error ) );
+        }
+        catch (const boost::bad_weak_ptr&)
+        {
+            // Object is not owned by shared_ptr (e.g., stack-allocated in tests)
+            // Use raw this pointer instead
+            SaveTimer.async_wait( bind( &DnsCache::schedule_save, this,
+                                        boost::asio::placeholders::error ) );
+        }
+    }
 }
 
 
 DnsCache::~DnsCache()
 {
-    GlobalLogger.info() << "DnsCache: being destructed";
+    // CRITICAL: Set InShutdown FIRST, before any logging or other operations.
+    // This ensures that any timer callbacks that might be triggered by other
+    // objects' destructors will see InShutdown=true and return early.
+    InShutdown = true;
 
-    // save one last time without re-scheduling the next save
-    save_to_cachefile();
+    // cancel save timer - pending callbacks will see InShutdown=true and return
+    boost::system::error_code ec;
+    SaveTimer.cancel(ec);
 
-    // cancel save timer
-    SaveTimer.cancel();
+    GlobalLogger.info() << "DnsCache: being destructed, InShutdown=" << InShutdown;
+
+    // save for caches that use a file - force save even though InShutdown is true
+    // Use the boolean flag to avoid comparing with potentially corrupted static string
+    if (UseCacheFile)
+    {
+        GlobalLogger.info() << "DnsCache: Will save cache file";
+        ForceSave = true;
+        save_to_cachefile();
+    }
+    else
+    {
+        GlobalLogger.info() << "DnsCache: Skipping save, not using cache file";
+    }
 }
 
 
@@ -97,29 +131,74 @@ DnsCache::~DnsCache()
 
 void DnsCache::schedule_save(const boost::system::error_code &error)
 {
+    // Skip if we're in shutdown mode - the destructor is running
+    // This MUST be checked FIRST to avoid accessing corrupted memory
+    // during destruction
+    if (InShutdown)
+    {
+        return;
+    }
+    // Skip if this cache doesn't use a file (like the global fixture in tests)
+    // Use boolean flag to avoid comparing with potentially corrupted static string
+    if (!UseCacheFile)
+    {
+        return;
+    }
     if ( error ==  boost::asio::error::operation_aborted )   // cancelled
     {
-        GlobalLogger.info() << "DnsCache: SaveTimer was cancelled "
-                            << "--> no save and no re-schedule of saving!";
         return;
     }
     else if (error)
     {
-        GlobalLogger.info() << "DnsCache: Received error " << error
-                            << " in schedule_save "
-                            << "--> no save now but re-schedule saving";
+        // schedule next save
+        (void) SaveTimer.expires_from_now( seconds( Config::SAVE_TIMER_SECONDS ) );
+        // Use try-catch for shared_from_this to support stack-allocated objects
+        try
+        {
+            SaveTimer.async_wait( bind( &DnsCache::schedule_save, shared_from_this(),
+                                        boost::asio::placeholders::error ) );
+        }
+        catch (const boost::bad_weak_ptr&)
+        {
+            // Object is not owned by shared_ptr (e.g., stack-allocated in tests)
+            // Don't reschedule - callback won't run after this point anyway
+        }
     }
     else
+    {
         save_to_cachefile();
 
-    // schedule next save
-    (void) SaveTimer.expires_from_now( seconds( Config::SAVE_TIMER_SECONDS ) );
-    SaveTimer.async_wait( bind( &DnsCache::schedule_save, this,
-                                boost::asio::placeholders::error ) );
+        // schedule next save
+        (void) SaveTimer.expires_from_now( seconds( Config::SAVE_TIMER_SECONDS ) );
+        // Use try-catch for shared_from_this to support stack-allocated objects
+        try
+        {
+            SaveTimer.async_wait( bind( &DnsCache::schedule_save, shared_from_this(),
+                                        boost::asio::placeholders::error ) );
+        }
+        catch (const boost::bad_weak_ptr&)
+        {
+            // Object is not owned by shared_ptr (e.g., stack-allocated in tests)
+            // Don't reschedule - callback won't run after this point anyway
+        }
+    }
 }
 
 void DnsCache::save_to_cachefile()
 {
+    // Skip entirely for caches that don't use a file (like the global fixture)
+    // Use boolean flag to avoid comparing with potentially corrupted static string
+    if (!UseCacheFile)
+    {
+        GlobalLogger.info() << "DnsCache::save_to_cachefile SKIPPED, UseCacheFile=false";
+        return;
+    }
+    GlobalLogger.info() << "DnsCache::save_to_cachefile called, InShutdown=" << InShutdown << ", ForceSave=" << ForceSave << ", CacheFile=" << CacheFile;
+    if (InShutdown && !ForceSave)
+    {
+        GlobalLogger.info() << "DnsCache: skip saving during shutdown";
+        return;
+    }
     if (!HasChanged)
         GlobalLogger.info() << "DnsCache: skip saving because has not changed";
     else if (CacheFile.empty())
@@ -152,6 +231,12 @@ void DnsCache::save_to_cachefile()
         {
             GlobalLogger.warning() << "DnsCache: Saving failed: " << exc.what();
         }
+        catch (...)
+        {
+            // Catch any other exceptions including assertion errors from
+            // Boost serialization during program shutdown
+            GlobalLogger.warning() << "DnsCache: Saving failed due to unknown error";
+        }
     }
 }
 
@@ -160,7 +245,7 @@ void DnsCache::load_from_cachefile()
     if (CacheFile.empty())
         GlobalLogger.info()
                    << "DnsCache: cannot load because cache file name is empty!";
-    else if (CacheFile == DoNotUseCacheFile)
+    else if (!UseCacheFile)
         GlobalLogger.info() << "DnsCache: configured not to use cache file";
     else if ( !I2n::file_exists(CacheFile) )
         GlobalLogger.warning() << "DnsCache: cannot load because cache file "
@@ -255,6 +340,23 @@ bool DnsCache::check_timestamps(const boost::posix_time::ptime &cache_save_time)
  */
 void DnsCache::remove_old_entries()
 {
+    // Skip if we're in shutdown mode and not forcing save (for destructor)
+    // or not using a file
+    // This prevents issues during program exit
+    // Use boolean flag to avoid comparing with potentially corrupted static string
+    if (!UseCacheFile)
+    {
+        GlobalLogger.debug() << "DnsCache::remove_old_entries skipped, UseCacheFile=false";
+        return;
+    }
+    if (InShutdown && !ForceSave)
+    {
+        GlobalLogger.debug() << "DnsCache::remove_old_entries skipped due to InShutdown=" << InShutdown << ", ForceSave=" << ForceSave;
+        return;
+    }
+
+    GlobalLogger.debug() << "DnsCache::remove_old_entries executing for " << CacheFile;
+
     boost::posix_time::ptime thresh
              = boost::posix_time::second_clock::universal_time()
              - boost::gregorian::days( Config::CACHE_REMOVE_OUTDATED_DAYS );

diff --git a/src/dns/dnscache.h b/src/dns/dnscache.h
index 91c1429..0ebf63e 100644 (file)
--- a/src/dns/dnscache.h
+++ b/src/dns/dnscache.h
@@ -25,7 +25,9 @@
 
 #include <map>
 
+#include <boost/enable_shared_from_this.hpp>
 #include <boost/shared_ptr.hpp>
+#include <boost/weak_ptr.hpp>
 #include <boost/asio/deadline_timer.hpp>
 #include <boost/system/error_code.hpp>
 #include <boost/archive/xml_oarchive.hpp>
@@ -47,7 +49,7 @@ typedef std::map<cname_map_key_type, Cname> cname_map_type;
 // DnsCache
 // -----------------------------------------------------------------------------
 
-class DnsCache
+class DnsCache : public boost::enable_shared_from_this<DnsCache>
 {
 public:
     /// constant to give as cache_file arg to DnsCache constructor
@@ -87,6 +89,9 @@ private:
     std::string CacheFile;
     bool HasChanged;
     uint32_t MinTimeBetweenResolves;
+    bool InShutdown;  // flag to skip saving during program shutdown
+    bool ForceSave;   // flag to force saving from destructor even when InShutdown
+    bool UseCacheFile;  // flag to indicate if we should use a cache file
 
 // internal functions
 private:
@@ -105,4 +110,3 @@ private:
 typedef boost::shared_ptr<DnsCache> DnsCacheItem;
 
 #endif
-

diff --git a/test/test_dns.cpp b/test/test_dns.cpp
index 8b35509..20802f3 100644 (file)
--- a/test/test_dns.cpp
+++ b/test/test_dns.cpp
@@ -103,8 +103,9 @@ struct GlobalFixture
     {
         BOOST_TEST_MESSAGE("Destructing global fixture");
         IoService->stop();
-        IoService.reset();
+        Cache.reset();
         Master.reset();
+        IoService.reset();
     }
 
     void fill_cache()