Fix race condition reading filename/string from config var

doc/ReleaseNotes

@@ -26,6 +26,7 @@ This issue fixes several bugs that were still found in 1.9.2.
 * Fix issues with certain Cg shader inputs in DX9
 * Support uint8 index buffers in DX9
 * Fix occasional frame lag when loading a big model asynchronously
+* Fix race condition reading string config var
 
 ------------------------  RELEASE 1.9.2  ------------------------
 

dtool/src/prc/configVariableFilename.cxx

@@ -23,16 +23,28 @@
 ////////////////////////////////////////////////////////////////////
 void ConfigVariableFilename::
 reload_cache() {
-  nassertv(_core != (ConfigVariableCore *)NULL);
-  mark_cache_valid(_local_modified);
+  // NB. MSVC doesn't guarantee that this mutex is initialized in a
+  // thread-safe manner.  But chances are that the first time this is called
+  // is at static init time, when there is no risk of data races.
+  static MutexImpl lock;
+  lock.acquire();
 
-  const ConfigDeclaration *decl = _core->get_declaration(0);
-  const ConfigPage *page = decl->get_page();
+  // We check again for cache validity since another thread may have beaten
+  // us to the punch while we were waiting for the lock.
+  if (!is_cache_valid(_local_modified)) {
+    nassertv(_core != (ConfigVariableCore *)NULL);
 
-  Filename page_filename(page->get_name());
-  Filename page_dirname = page_filename.get_dirname();
-  ExecutionEnvironment::shadow_environment_variable("THIS_PRC_DIR", page_dirname.to_os_specific());
+    const ConfigDeclaration *decl = _core->get_declaration(0);
+    const ConfigPage *page = decl->get_page();
 
-  _cache = Filename::expand_from(decl->get_string_value());
-  ExecutionEnvironment::clear_shadow("THIS_PRC_DIR");
+    Filename page_filename(page->get_name());
+    Filename page_dirname = page_filename.get_dirname();
+    ExecutionEnvironment::shadow_environment_variable("THIS_PRC_DIR", page_dirname.to_os_specific());
+
+    _cache = Filename::expand_from(decl->get_string_value());
+    ExecutionEnvironment::clear_shadow("THIS_PRC_DIR");
+
+    mark_cache_valid(_local_modified);
+  }
+  lock.release();
 }

dtool/src/prc/configVariableString.I

@@ -155,8 +155,7 @@ INLINE const string &ConfigVariableString::
 get_value() const {
   TAU_PROFILE("const string &ConfigVariableString::get_value() const", " ", TAU_USER);
   if (!is_cache_valid(_local_modified)) {
-    mark_cache_valid(((ConfigVariableString *)this)->_local_modified);
-    ((ConfigVariableString *)this)->_cache = get_string_value();
+    ((ConfigVariableString *)this)->reload_cache();
   }
   return _cache;
 }

dtool/src/prc/configVariableString.cxx

@@ -13,3 +13,26 @@
 ////////////////////////////////////////////////////////////////////
 
 #include "configVariableString.h"
+
+////////////////////////////////////////////////////////////////////
+//     Function: ConfigVariableString::reload_cache
+//       Access: Published
+//  Description: Refreshes the variable's cached value.
+////////////////////////////////////////////////////////////////////
+void ConfigVariableString::
+reload_cache() {
+  // NB. MSVC doesn't guarantee that this mutex is initialized in a
+  // thread-safe manner.  But chances are that the first time this is called
+  // is at static init time, when there is no risk of data races.
+  static MutexImpl lock;
+  lock.acquire();
+
+  // We check again for cache validity since another thread may have beaten
+  // us to the punch while we were waiting for the lock.
+  if (!is_cache_valid(_local_modified)) {
+    _cache = get_string_value();
+    mark_cache_valid(_local_modified);
+  }
+
+  lock.release();
+}

dtool/src/prc/configVariableString.h

@@ -51,6 +51,9 @@ PUBLISHED:
   INLINE string get_word(int n) const;
   INLINE void set_word(int n, const string &value);
 
+private:
+  void reload_cache();
+
 private:
   AtomicAdjust::Integer _local_modified;
   string _cache;