diff --git a/doc/ReleaseNotes b/doc/ReleaseNotes index 3796b840c0..94ebbe4e9e 100644 --- a/doc/ReleaseNotes +++ b/doc/ReleaseNotes @@ -25,6 +25,8 @@ This issue fixes several bugs that were still found in 1.9.2. * Work around Cg bug generating invalid ASM for saturated tex loads * 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 dc82a88f45..3939199148 100644 --- a/dtool/src/prc/configVariableFilename.cxx +++ b/dtool/src/prc/configVariableFilename.cxx @@ -19,16 +19,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 b3b88b90fe..1adbbe6797 100644 --- a/dtool/src/prc/configVariableString.I +++ b/dtool/src/prc/configVariableString.I @@ -127,8 +127,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 125c8841a9..3dcaaf6deb 100644 --- a/dtool/src/prc/configVariableString.cxx +++ b/dtool/src/prc/configVariableString.cxx @@ -12,3 +12,24 @@ */ #include "configVariableString.h" + +/** + * 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 08709e1ac1..1592e0b836 100644 --- a/dtool/src/prc/configVariableString.h +++ b/dtool/src/prc/configVariableString.h @@ -49,6 +49,9 @@ PUBLISHED: INLINE string get_word(size_t n) const; INLINE void set_word(size_t n, const string &value); +private: + void reload_cache(); + private: AtomicAdjust::Integer _local_modified; string _cache; diff --git a/panda/src/putil/bamCache.cxx b/panda/src/putil/bamCache.cxx index 06211b5690..5576e16a43 100644 --- a/panda/src/putil/bamCache.cxx +++ b/panda/src/putil/bamCache.cxx @@ -310,13 +310,24 @@ emergency_read_only() { */ void BamCache:: consider_flush_index() { - ReMutexHolder holder(_lock); +#if defined(HAVE_THREADS) || defined(DEBUG_THREADS) + if (!_lock.try_acquire()) { + // If we can't grab the lock, no big deal. We don't want to hold up + // the frame waiting for a cache operation. We can try again later. + return; + } +#endif + if (_index_stale_since != 0) { int elapsed = (int)time(NULL) - (int)_index_stale_since; if (elapsed > _flush_time) { flush_index(); } } + +#if defined(HAVE_THREADS) || defined(DEBUG_THREADS) + _lock.release(); +#endif } /**