QTzTimeZoneCache: don't hold mutex while parsing files

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
Pick-to: 6.7
Change-Id: If0cba6d041fb21a48cbde6b43190662a2c55cd25
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Matthias Rauter <matthias.rauter@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

1 files changed, 11 insertions, 1 deletions

diff --git a/src/corelib/time/qtimezoneprivate_tz.cpp b/src/corelib/time/qtimezoneprivate_tz.cpp
index 7d1655b88f..f4a2df00a4 100644
--- a/src/corelib/time/qtimezoneprivate_tz.cpp
+++ b/src/corelib/time/qtimezoneprivate_tz.cpp
@@ -20,6 +20,8 @@
 #include <qplatformdefs.h>
 
 #include <algorithm>
+#include <memory>
+
 #include <errno.h>
 #include <limits.h>
 #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<QTzTimeZoneCacheEntry>(ret);
+
+    locker.relock();
+    m_cache.insert(ianaId, ptr.release()); // may overwrite if another thread was faster
+    locker.unlock();
+
     return ret;
 }