Skip to content

Commit

Permalink
QTzTimeZoneCache: don't hold mutex while parsing files
Browse files Browse the repository at this point in the history
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 <edward.welbourne@qt.io>
Reviewed-by: Matthias Rauter <matthias.rauter@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 039b2e4)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
  • Loading branch information
marcmutz authored and Qt Cherry-pick Bot committed Feb 18, 2024
1 parent 3265e24 commit 21177c7
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion src/corelib/time/qtimezoneprivate_tz.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include <qplatformdefs.h>

#include <algorithm>
#include <memory>

#include <errno.h>
#include <limits.h>
#ifndef Q_OS_INTEGRITY
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit 21177c7

Please sign in to comment.