diff --git a/doc/ReleaseNotes b/doc/ReleaseNotes index a4ca29a6cf..94ebbe4e9e 100644 --- a/doc/ReleaseNotes +++ b/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 ------------------------ diff --git a/dtool/src/prc/configVariableFilename.cxx b/dtool/src/prc/configVariableFilename.cxx index dd9a4ab3ef..2b898f5b11 100644 --- a/dtool/src/prc/configVariableFilename.cxx +++ b/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(); } diff --git a/dtool/src/prc/configVariableString.I b/dtool/src/prc/configVariableString.I index 580ad91fd3..2982b2ca24 100644 --- a/dtool/src/prc/configVariableString.I +++ b/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; } diff --git a/dtool/src/prc/configVariableString.cxx b/dtool/src/prc/configVariableString.cxx index e1b7796f3f..7f4c3048bf 100644 --- a/dtool/src/prc/configVariableString.cxx +++ b/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(); +} diff --git a/dtool/src/prc/configVariableString.h b/dtool/src/prc/configVariableString.h index 0685af8a67..f8b4e51c9d 100644 --- a/dtool/src/prc/configVariableString.h +++ b/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;