Fix destruction order in NodeManager

Fixes #4576. ASan was reporting memory leaks because the skolem manager
was being destroyed after the attributes and zombies were already
cleaned up in the destructor of NodeManager. This commit changes the
destruction order to ensure that the skolem manager is destroyed before
the rest of the cleanup.

Note: this issue did not only make the benchmark in #4576 fail but
several tests in our regressions.

1 files changed, 3 insertions, 1 deletions

diff --git a/src/expr/node_manager.cpp b/src/expr/node_manager.cpp
index 6f8934645..b32c93313 100644
--- a/src/expr/node_manager.cpp
+++ b/src/expr/node_manager.cpp
@@ -171,6 +171,9 @@ NodeManager::~NodeManager() {
 
   NodeManagerScope nms(this);
 
+  // Destroy skolem manager before cleaning up attributes and zombies
+  d_skManager = nullptr;
+
   {
     ScopedBool dontGC(d_inReclaimZombies);
     // hopefully by this point all SmtEngines have been deleted
@@ -233,7 +236,6 @@ NodeManager::~NodeManager() {
   // defensive coding, in case destruction-order issues pop up (they often do)
   delete d_resourceManager;
   d_resourceManager = NULL;
-  d_skManager = nullptr;
   delete d_statisticsRegistry;
   d_statisticsRegistry = NULL;
   delete d_registrations;