Browse Source

better error reporting for static init ordering issues w.r.t config variables

David Rose 16 years ago
parent
commit
5a74ea30dd

+ 40 - 20
dtool/src/prc/configVariable.I

@@ -71,7 +71,7 @@ INLINE ConfigVariable::
 ////////////////////////////////////////////////////////////////////
 INLINE const ConfigDeclaration *ConfigVariable::
 get_default_value() const {
-  nassertr(_core != (ConfigVariableCore *)NULL, (ConfigDeclaration *)NULL);
+  nassertr(is_constructed(), (ConfigDeclaration *)NULL);
   return _core->get_default_value();
 }
 
@@ -83,7 +83,7 @@ get_default_value() const {
 ////////////////////////////////////////////////////////////////////
 INLINE const string &ConfigVariable::
 get_string_value() const {
-  nassertr(_core != (ConfigVariableCore *)NULL, *new string());
+  nassertr(is_constructed(), *new string());
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->get_string_value();
 }
@@ -98,7 +98,7 @@ get_string_value() const {
 ////////////////////////////////////////////////////////////////////
 INLINE void ConfigVariable::
 set_string_value(const string &string_value) {
-  nassertv(_core != (ConfigVariableCore *)NULL);
+  nassertv(is_constructed());
   _core->make_local_value()->set_string_value(string_value);
 }
 
@@ -111,7 +111,7 @@ set_string_value(const string &string_value) {
 ////////////////////////////////////////////////////////////////////
 INLINE void ConfigVariable::
 clear_value() {
-  nassertv(_core != (ConfigVariableCore *)NULL);
+  nassertv(is_constructed());
   _core->clear_local_value();
 }
 
@@ -124,7 +124,7 @@ clear_value() {
 ////////////////////////////////////////////////////////////////////
 INLINE int ConfigVariable::
 get_num_words() const {
-  nassertr(_core != (ConfigVariableCore *)NULL, 0);
+  nassertr(is_constructed(), 0);
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->get_num_words();
 }
@@ -139,7 +139,7 @@ get_num_words() const {
 ////////////////////////////////////////////////////////////////////
 INLINE bool ConfigVariable::
 has_string_word(int n) const {
-  nassertr(_core != (ConfigVariableCore *)NULL, false);
+  nassertr(is_constructed(), false);
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->has_string_word(n);
 }
@@ -152,7 +152,7 @@ has_string_word(int n) const {
 ////////////////////////////////////////////////////////////////////
 INLINE bool ConfigVariable::
 has_bool_word(int n) const {
-  nassertr(_core != (ConfigVariableCore *)NULL, false);
+  nassertr(is_constructed(), false);
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->has_bool_word(n);
 }
@@ -165,7 +165,7 @@ has_bool_word(int n) const {
 ////////////////////////////////////////////////////////////////////
 INLINE bool ConfigVariable::
 has_int_word(int n) const {
-  nassertr(_core != (ConfigVariableCore *)NULL, false);
+  nassertr(is_constructed(), false);
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->has_int_word(n);
 }
@@ -178,7 +178,7 @@ has_int_word(int n) const {
 ////////////////////////////////////////////////////////////////////
 INLINE bool ConfigVariable::
 has_int64_word(int n) const {
-  nassertr(_core != (ConfigVariableCore *)NULL, false);
+  nassertr(is_constructed(), false);
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->has_int64_word(n);
 }
@@ -191,7 +191,7 @@ has_int64_word(int n) const {
 ////////////////////////////////////////////////////////////////////
 INLINE bool ConfigVariable::
 has_double_word(int n) const {
-  nassertr(_core != (ConfigVariableCore *)NULL, false);
+  nassertr(is_constructed(), false);
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->has_double_word(n);
 }
@@ -205,7 +205,7 @@ has_double_word(int n) const {
 ////////////////////////////////////////////////////////////////////
 INLINE string ConfigVariable::
 get_string_word(int n) const {
-  nassertr(_core != (ConfigVariableCore *)NULL, string());
+  nassertr(is_constructed(), string());
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->get_string_word(n);
 }
@@ -219,7 +219,7 @@ get_string_word(int n) const {
 ////////////////////////////////////////////////////////////////////
 INLINE bool ConfigVariable::
 get_bool_word(int n) const {
-  nassertr(_core != (ConfigVariableCore *)NULL, false);
+  nassertr(is_constructed(), false);
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->get_bool_word(n);
 }
@@ -233,7 +233,7 @@ get_bool_word(int n) const {
 ////////////////////////////////////////////////////////////////////
 INLINE int ConfigVariable::
 get_int_word(int n) const {
-  nassertr(_core != (ConfigVariableCore *)NULL, 0);
+  nassertr(is_constructed(), 0);
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->get_int_word(n);
 }
@@ -247,7 +247,7 @@ get_int_word(int n) const {
 ////////////////////////////////////////////////////////////////////
 INLINE PN_int64 ConfigVariable::
 get_int64_word(int n) const {
-  nassertr(_core != (ConfigVariableCore *)NULL, 0);
+  nassertr(is_constructed(), 0);
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->get_int64_word(n);
 }
@@ -261,7 +261,7 @@ get_int64_word(int n) const {
 ////////////////////////////////////////////////////////////////////
 INLINE double ConfigVariable::
 get_double_word(int n) const {
-  nassertr(_core != (ConfigVariableCore *)NULL, 0.0);
+  nassertr(is_constructed(), 0.0);
   const ConfigDeclaration *decl = _core->get_declaration(0);
   return decl->get_double_word(n);
 }
@@ -274,7 +274,7 @@ get_double_word(int n) const {
 ////////////////////////////////////////////////////////////////////
 INLINE void ConfigVariable::
 set_string_word(int n, const string &value) {
-  nassertv(_core != (ConfigVariableCore *)NULL);
+  nassertv(is_constructed());
   _core->make_local_value()->set_string_word(n, value);
 }
 
@@ -286,7 +286,7 @@ set_string_word(int n, const string &value) {
 ////////////////////////////////////////////////////////////////////
 INLINE void ConfigVariable::
 set_bool_word(int n, bool value) {
-  nassertv(_core != (ConfigVariableCore *)NULL);
+  nassertv(is_constructed());
   _core->make_local_value()->set_bool_word(n, value);
 }
 
@@ -298,7 +298,7 @@ set_bool_word(int n, bool value) {
 ////////////////////////////////////////////////////////////////////
 INLINE void ConfigVariable::
 set_int_word(int n, int value) {
-  nassertv(_core != (ConfigVariableCore *)NULL);
+  nassertv(is_constructed());
   _core->make_local_value()->set_int_word(n, value);
 }
 
@@ -310,7 +310,7 @@ set_int_word(int n, int value) {
 ////////////////////////////////////////////////////////////////////
 INLINE void ConfigVariable::
 set_int64_word(int n, PN_int64 value) {
-  nassertv(_core != (ConfigVariableCore *)NULL);
+  nassertv(is_constructed());
   _core->make_local_value()->set_int_word(n, value);
 }
 
@@ -322,6 +322,26 @@ set_int64_word(int n, PN_int64 value) {
 ////////////////////////////////////////////////////////////////////
 INLINE void ConfigVariable::
 set_double_word(int n, double value) {
-  nassertv(_core != (ConfigVariableCore *)NULL);
+  nassertv(is_constructed());
   _core->make_local_value()->set_double_word(n, value);
 }
+
+////////////////////////////////////////////////////////////////////
+//     Function: ConfigVariable::is_constructed
+//       Access: Protected
+//  Description: Returns true if the constructor has been called and
+//               _core initialized, false if the constructor has not
+//               yet been called and _core is NULL.  This is intended
+//               to be placed in an assertion check, to guard against
+//               static-init ordering issues.
+////////////////////////////////////////////////////////////////////
+INLINE bool ConfigVariable::
+is_constructed() const {
+#ifndef NDEBUG
+  if (_core == (ConfigVariableCore *)NULL) {
+    report_unconstructed();
+    return false;
+  }
+#endif
+  return true;
+}

+ 17 - 0
dtool/src/prc/configVariable.cxx

@@ -13,3 +13,20 @@
 ////////////////////////////////////////////////////////////////////
 
 #include "configVariable.h"
+#include "config_prc.h"
+
+////////////////////////////////////////////////////////////////////
+//     Function: ConfigVariable::report_unconstructed
+//       Access: Protected
+//  Description: Displays a suitable error message when an
+//               unconstructed ConfigVariable is attempted to be used.
+//               This normally indicates a static-init ordering issue.
+////////////////////////////////////////////////////////////////////
+void ConfigVariable::
+report_unconstructed() const {
+  prc_cat->error()
+    << "ConfigVariable " << this
+    << " accessed before its constructor has run!\n";
+  record_unconstructed();
+}
+

+ 4 - 0
dtool/src/prc/configVariable.h

@@ -67,6 +67,10 @@ PUBLISHED:
   INLINE void set_int_word(int n, int value);
   INLINE void set_int64_word(int n, PN_int64 value);
   INLINE void set_double_word(int n, double value);
+
+protected:
+  INLINE bool is_constructed() const;
+  void report_unconstructed() const;
 };
 
 #include "configVariable.I"

+ 49 - 0
dtool/src/prc/configVariableBase.cxx

@@ -14,6 +14,8 @@
 
 #include "configVariableBase.h"
 
+ConfigVariableBase::Unconstructed *ConfigVariableBase::_unconstructed;
+
 ////////////////////////////////////////////////////////////////////
 //     Function: ConfigVariableBase::Constructor
 //       Access: Protected
@@ -26,6 +28,13 @@ ConfigVariableBase(const string &name,
                    const string &description, int flags) :
   _core(ConfigVariableManager::get_global_ptr()->make_variable(name))
 {
+#ifndef NDEBUG
+  if (was_unconstructed()) {
+    prc_cat->error()
+      << "Late constructing " << this << ": " << name << "\n";
+  }
+#endif  // NDEBUG
+
   if (value_type != VT_undefined) {
     _core->set_value_type(value_type);
   }
@@ -38,3 +47,43 @@ ConfigVariableBase(const string &name,
     _core->set_flags(flags);
   }
 }
+
+////////////////////////////////////////////////////////////////////
+//     Function: ConfigVariableBase::record_unconstructed
+//       Access: Protected
+//  Description: Records that this config variable was referenced
+//               before it was constructed (presumably a static-init
+//               ordering issue).  This is used to print a useful
+//               error message later, when the constructor is actually
+//               called (and we then know what the name of the
+//               variable is).
+////////////////////////////////////////////////////////////////////
+void ConfigVariableBase::
+record_unconstructed() const {
+#ifndef NDEBUG
+  if (_unconstructed == (Unconstructed *)NULL) {
+    _unconstructed = new Unconstructed;
+  }
+  _unconstructed->insert(this);
+#endif
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: ConfigVariableBase::was_unconstructed
+//       Access: Protected
+//  Description: Returns true if record_unconstructed() was ever
+//               called on this pointer, false otherwise.
+////////////////////////////////////////////////////////////////////
+bool ConfigVariableBase::
+was_unconstructed() const {
+#ifndef NDEBUG
+  if (_unconstructed != (Unconstructed *)NULL) {
+    Unconstructed::const_iterator ui = _unconstructed->find(this);
+    if (ui != _unconstructed->end()) {
+      return true;
+    }
+  }
+#endif
+  return false;
+}
+

+ 7 - 0
dtool/src/prc/configVariableBase.h

@@ -21,6 +21,7 @@
 #include "configDeclaration.h"
 #include "configVariableManager.h"
 #include "vector_string.h"
+#include "pset.h"
 
 // Use this macro to wrap around a description passed to a
 // ConfigVariable constructor.  This allows the description to be
@@ -69,7 +70,13 @@ PUBLISHED:
   INLINE void write(ostream &out) const;
 
 protected:
+  void record_unconstructed() const;
+  bool was_unconstructed() const;
+
   ConfigVariableCore *_core;
+
+  typedef pset<const ConfigVariableBase *> Unconstructed;
+  static Unconstructed *_unconstructed;
 };
 
 INLINE ostream &operator << (ostream &out, const ConfigVariableBase &variable);

+ 12 - 1
dtool/src/prc/configVariableCore.cxx

@@ -181,7 +181,18 @@ set_description(const string &description) {
       }
       return;
     }
-      
+
+    if (description.empty()) {
+      // If the new description is empty, we don't do anything.
+      return;
+    }
+
+    if (_description.empty()) {
+      // If the previous description was empty, we quietly replace it.
+      _description = description;
+      return;
+    }
+
     prc_cat->warning()
       << "changing description for ConfigVariable " 
       << get_name() << ".\n";

+ 3 - 0
dtool/src/prc/notify.cxx

@@ -437,6 +437,9 @@ assert_failure(const char *expression, int line,
 
   nout << "Assertion failed: " << message << "\n";
 
+  // This is redefined here, shadowing the defining in config_prc.h,
+  // so we can guarantee it has already been constructed.
+  ConfigVariableBool assert_abort("assert-abort", false);
   if (assert_abort) {
 #ifdef WIN32
     // How to trigger an exception in VC++ that offers to take us into