Avoid races on TZ in environment via tzset() calls

POSIX specifies that tzset() consults environment variable TZ and
modifies some globals; it also specifies mktime(), localtime() and
strftime() to behave as if they called tzset().  Fortunately, we only
call strftime() from a test and only call localtime() when not
threaded.  Provide wrappers for tzset() and mktime() that share the
lock used by our environment-access code, to prevent races on the
environment (and tzset()'s globals) when we call them.

In the process, re-use tst_QDateTime's TimeZoneRollback in its older
test systemTimeZoneChange() and presume that this can now be tested
cross-platform, since TimeZoneRollback is used in another test where
this works.

Fixes: QTBUG-71030
Change-Id: I79f559b8857ea2803e73501008bf0d7158c6731f
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/global/qglobal.cpp

@@ -3252,6 +3252,34 @@ void *qMemSet(void *dest, int c, size_t n) { return memset(dest, c, n); }
 // add thread-safety for the Qt wrappers.
 static QBasicMutex environmentMutex;
 
+/*
+  Wraps tzset(), which accesses the environment, so should only be called while
+  we hold the lock on the environment mutex.
+*/
+void qTzSet()
+{
+    QMutexLocker locker(&environmentMutex);
+#if defined(Q_OS_WIN)
+    _tzset();
+#else
+    tzset();
+#endif // Q_OS_WIN
+}
+
+/*
+  Wrap mktime(), which is specified to behave as if it called tzset(), hence
+  shares its implicit environment-dependence.
+*/
+time_t qMkTime(struct tm *when)
+{
+    QMutexLocker locker(&environmentMutex);
+    return mktime(when);
+}
+
+// Also specified to behave as if they call tzset():
+// localtime() -- but not localtime_r(), which we use when threaded
+// strftime() -- not used (except in tests)
+
 /*!
     \relates <QtGlobal>
     \threadsafe

src/corelib/global/qglobal_p.h

@@ -61,6 +61,16 @@
 #endif
 
 #if defined(__cplusplus)
+#include <time.h>
+
+QT_BEGIN_NAMESPACE
+
+// These behave as if they consult the environment, so need to share its locking:
+Q_CORE_EXPORT void qTzSet();
+Q_CORE_EXPORT time_t qMkTime(struct tm *when);
+
+QT_END_NAMESPACE
+
 #if !QT_HAS_BUILTIN(__builtin_available)
 #include <initializer_list>
 #include <QtCore/qoperatingsystemversion.h>

src/corelib/tools/qdatetime.cpp

@@ -2171,16 +2171,6 @@ int QTime::elapsed() const
 typedef QDateTimePrivate::QDateTimeShortData ShortData;
 typedef QDateTimePrivate::QDateTimeData QDateTimeData;
 
-// Calls the platform variant of tzset
-static void qt_tzset()
-{
-#if defined(Q_OS_WIN)
-    _tzset();
-#else
-    tzset();
-#endif // Q_OS_WIN
-}
-
 // Returns the platform variant of timezone, i.e. the standard time offset
 // The timezone external variable is documented as always holding the
 // Standard Time offset as seconds west of Greenwich, i.e. UTC+01:00 is -3600
@@ -2278,7 +2268,7 @@ static qint64 qt_mktime(QDate *date, QTime *time, QDateTimePrivate::DaylightStat
 #if defined(Q_OS_WIN)
     int hh = local.tm_hour;
 #endif // Q_OS_WIN
-    time_t secsSinceEpoch = mktime(&local);
+    time_t secsSinceEpoch = qMkTime(&local);
     if (secsSinceEpoch != time_t(-1)) {
         *date = QDate(local.tm_year + 1900, local.tm_mon + 1, local.tm_mday);
         *time = QTime(local.tm_hour, local.tm_min, local.tm_sec, msec);
@@ -2337,10 +2327,10 @@ static bool qt_localtime(qint64 msecsSinceEpoch, QDate *localDate, QTime *localT
     tm local;
     bool valid = false;
 
-    // localtime() is required to work as if tzset() was called before it.
-    // localtime_r() does not have this requirement, so make an explicit call.
+    // localtime() is specified to work as if it called tzset().
+    // localtime_r() does not have this constraint, so make an explicit call.
     // The explicit call should also request the timezone info be re-parsed.
-    qt_tzset();
+    qTzSet();
 #if QT_CONFIG(thread) && defined(_POSIX_THREAD_SAFE_FUNCTIONS)
     // Use the reentrant version of localtime() where available
     // as is thread-safe and doesn't use a shared static data area
@@ -2422,7 +2412,7 @@ static bool epochMSecsToLocalTime(qint64 msecs, QDate *localDate, QTime *localTi
     if (msecs < 0) {
         // Docs state any LocalTime before 1970-01-01 will *not* have any Daylight Time applied
         // Instead just use the standard offset from UTC to convert to UTC time
-        qt_tzset();
+        qTzSet();
         msecsToTime(msecs - qt_timezone() * 1000, localDate, localTime);
         if (daylightStatus)
             *daylightStatus = QDateTimePrivate::StandardTime;
@@ -2485,7 +2475,7 @@ static qint64 localMSecsToEpochMSecs(qint64 localMsecs,
             }
         } else {
             // If we don't call mktime then need to call tzset to get offset
-            qt_tzset();
+            qTzSet();
         }
         // Time is clearly before 1970-01-01 so just use standard offset to convert
         qint64 utcMsecs = localMsecs + qt_timezone() * 1000;

tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp

@@ -37,9 +37,6 @@
 
 #ifdef Q_OS_WIN
 #   include <qt_windows.h>
-#  if defined(Q_OS_WINRT)
-#    define tzset()
-#  endif
 #endif
 
 class tst_QDateTime : public QObject
@@ -145,9 +142,7 @@ private slots:
     void isDaylightTime() const;
     void daylightTransitions() const;
     void timeZones() const;
-#if defined(Q_OS_UNIX)
     void systemTimeZoneChange() const;
-#endif
 
     void invalid() const;
 
@@ -174,7 +169,7 @@ private:
         void reset(const QByteArray &zone)
         {
             qputenv("TZ", zone.constData());
-            tzset();
+            qTzSet();
         }
         ~TimeZoneRollback()
         {
@@ -182,7 +177,7 @@ private:
                 qunsetenv("TZ");
             else
                 qputenv("TZ", prior.constData());
-            tzset();
+            qTzSet();
         }
     };
 };
@@ -3412,33 +3407,10 @@ void tst_QDateTime::timeZones() const
     QCOMPARE(future.offsetFromUtc(), 28800);
 }
 
-#if defined(Q_OS_UNIX)
-// Currently disabled on Windows as adjusting the timezone
-// requires additional privileges that aren't normally
-// enabled for a process. This can be achieved by calling
-// AdjustTokenPrivileges() and then SetTimeZoneInformation(),
-// which will require linking to a different library to access that API.
-static void setTimeZone(const QByteArray &tz)
-{
-    qputenv("TZ", tz);
-    ::tzset();
-
-// following left for future reference, see comment above
-// #if defined(Q_OS_WIN32)
-//     ::_tzset();
-// #endif
-}
-
 void tst_QDateTime::systemTimeZoneChange() const
 {
-    struct ResetTZ {
-        QByteArray original;
-        ResetTZ() : original(qgetenv("TZ")) {}
-        ~ResetTZ() { setTimeZone(original); }
-    } scopedReset;
-
     // Set the timezone to Brisbane time
-    setTimeZone(QByteArray("AEST-10:00"));
+    TimeZoneRollback useZone(QByteArray("AEST-10:00"));
 
     QDateTime localDate = QDateTime(QDate(2012, 6, 1), QTime(2, 15, 30), Qt::LocalTime);
     QDateTime utcDate = QDateTime(QDate(2012, 6, 1), QTime(2, 15, 30), Qt::UTC);
@@ -3451,16 +3423,18 @@ void tst_QDateTime::systemTimeZoneChange() const
     QVERIFY(tzDate.timeZone().isValid());
 
     // Change to Indian time
-    setTimeZone(QByteArray("IST-05:30"));
+    useZone.reset(QByteArray("IST-05:30"));
 
     QCOMPARE(localDate, QDateTime(QDate(2012, 6, 1), QTime(2, 15, 30), Qt::LocalTime));
+#ifdef Q_OS_WINRT
+    QEXPECT_FAIL("", "WinRT gets this wrong, QTBUG-71185", Continue);
+#endif
     QVERIFY(localMsecs != localDate.toMSecsSinceEpoch());
     QCOMPARE(utcDate, QDateTime(QDate(2012, 6, 1), QTime(2, 15, 30), Qt::UTC));
     QCOMPARE(utcDate.toMSecsSinceEpoch(), utcMsecs);
     QCOMPARE(tzDate, QDateTime(QDate(2012, 6, 1), QTime(2, 15, 30), QTimeZone("Australia/Brisbane")));
     QCOMPARE(tzDate.toMSecsSinceEpoch(), tzMsecs);
 }
-#endif
 
 void tst_QDateTime::invalid() const
 {