From 21177c79bc87a433295d327f1567b4e7c7ed534d Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 9 Feb 2024 18:47:02 +0100 Subject: [PATCH] QTzTimeZoneCache: don't hold mutex while parsing files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The QTzTimeZoneCache::findEntry() function is always called with QTzTimeZoneCache::m_mutex held, from its only caller, QTzTimeZoneCache::fetchEntry(). However, findEntry() performs quite heavy parsing, reading e.g. /etc/localtime or a file in /usr/share/zoneinfo/. These files are larger than 2KiB file on my system. Even though findEntry() doesn't touch m_cache during its operation¹, it thus prevents other threads from looking up (and even parsing) other entries in the cache. A straight-forward solution is therefore to drop the mutex in fetchEntry() for the duration of the findEntry() call² and then re-acquire it for the final m_cache.insert(). This means, of course, that more than one thread could parse the same file concurrently, and then one of the thread's result would be discarded. Having the file already in the OS cache makes this probably less of an issue than it sounds, but more complicated protocols are readily implementable, should it become necessary. QTBUG-122138 has a sketch. ¹ It's a static function, so it cannot access NSDMs. ² Incl. the heap allocation required to store the result in QCache. Fixes: QTBUG-122138 Change-Id: If0cba6d041fb21a48cbde6b43190662a2c55cd25 Reviewed-by: Edward Welbourne Reviewed-by: Matthias Rauter Reviewed-by: Thiago Macieira (cherry picked from commit 039b2e4e5dca0fbf98a10fe548467034eeddf117) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/time/qtimezoneprivate_tz.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/corelib/time/qtimezoneprivate_tz.cpp b/src/corelib/time/qtimezoneprivate_tz.cpp index e68a7d4726a..c682fc8f70d 100644 --- a/src/corelib/time/qtimezoneprivate_tz.cpp +++ b/src/corelib/time/qtimezoneprivate_tz.cpp @@ -20,6 +20,8 @@ #include #include +#include + #include #include #ifndef Q_OS_INTEGRITY @@ -980,8 +982,16 @@ QTzTimeZoneCacheEntry QTzTimeZoneCache::fetchEntry(const QByteArray &ianaId) return *obj; // ... or build a new entry from scratch + + locker.unlock(); // don't parse files under mutex lock + QTzTimeZoneCacheEntry ret = findEntry(ianaId); - m_cache.insert(ianaId, new QTzTimeZoneCacheEntry(ret)); + auto ptr = std::make_unique(ret); + + locker.relock(); + m_cache.insert(ianaId, ptr.release()); // may overwrite if another thread was faster + locker.unlock(); + return ret; }