Make attributes robust to static init orderings (#2295)

@taking pointed out that part of the issue fixed in #2293 is also that
we should be more robust to different (de-)initialization orders. A
common, portable way to achieve this is to allocate the object in
question on the heap and make the pointer to it static [0]. This commit
fixes the variable in question.

I have tested this fix in ASAN (without using --no-static-init flag for
CxxTest) and it works.

[0] https://isocpp.org/wiki/faq/ctors#construct-on-first-use-v2

diff --git a/src/expr/attribute_internals.h b/src/expr/attribute_internals.h
index 51116963d5ef8a3a7d9a18fe95a92c2f0b240d7c..c21b151408bd25b1983d0bd494f1ef3d0a968aab 100644 (file)
--- a/src/expr/attribute_internals.h
+++ b/src/expr/attribute_internals.h
@@ -505,8 +505,19 @@ template <class T, bool context_dep>
 struct AttributeTraits {
   typedef void (*cleanup_t)(T);
   static std::vector<cleanup_t>& getCleanup() {
-    static std::vector<cleanup_t> cleanup;
-    return cleanup;
+    // Note: we do not destroy this vector on purpose. Instead, we rely on the
+    // OS to clean up our mess. The reason for that is that we need this vector
+    // to remain initialized at least as long as the ExprManager because
+    // ExprManager's destructor calls this method. The only way to guarantee
+    // this is to never destroy it. This is a common idiom [0]. In the past, we
+    // had an issue when `cleanup` wasn't a pointer and just an `std::vector`
+    // instead. CxxTest stores the test class in a static variable, which could
+    // lead to the ExprManager being destroyed after the destructor of the
+    // vector was called.
+    //
+    // [0] https://isocpp.org/wiki/faq/ctors#static-init-order-on-first-use
+    static std::vector<cleanup_t>* cleanup = new std::vector<cleanup_t>();
+    return *cleanup;
   }
 };