AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix race condition in QDateTime::timeZone() and other methods

Browse Source 
When timezone support for QDateTime was added, we decided it was a good
idea to delay creating the QTimeZone object and checking that the time
is valid in that timezone (including for local time) until the user
requested that information. Unfortunately, QExplicitlySharedDataPointer
returns a non-const T* in operator->(), which meant we were accidentally
modifying the d pointer's contents in const methods, which in turn means
those const methods were not thread-safe when operating on the same
object.

This commit changes the d pointer to QSharedDataPointer, which is safer
in this regard and pointed out where the issues with constness were
located. Since we can't lazily calculate QTimeZone anymore, we need to
do it whenever the date, time or offset changes.

Task-number: QTBUG-43703
Change-Id: Ic5d393bfd36e48a193fcffff13b9686ef4ef1454
Reviewed-by: Lars Knoll <lars.knoll@digia.com>
This commit is contained in:
Thiago Macieira 2015-01-14 16:33:03 -08:00
parent 84b9d07163
commit a8c74ddcf7
4 changed files with 89 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
src/corelib/tools/qdatetime.cpp
View File

@ -2607,7 +2607,6 @@ QDateTimePrivate::QDateTimePrivate(const QDate &toDate, const QTime &toTime,
void QDateTimePrivate::setTimeSpec(Qt::TimeSpec spec, int offsetSeconds)
{
clearValidDateTime();
clearTimeZoneCached();
clearSetToDaylightStatus();
#ifndef QT_BOOTSTRAPPED
@ -2707,27 +2706,44 @@ QDateTimePrivate::DaylightStatus QDateTimePrivate::daylightStatus() const
return UnknownDaylightTime;
}
qint64 QDateTimePrivate::toMSecsSinceEpoch() const
{
switch (m_spec) {
case Qt::OffsetFromUTC:
case Qt::UTC:
return (m_msecs - (m_offsetFromUtc * 1000));
case Qt::LocalTime:
// recalculate the local timezone
return localMSecsToEpochMSecs(m_msecs);
case Qt::TimeZone:
#ifndef QT_BOOTSTRAPPED
return zoneMSecsToEpochMSecs(m_msecs, m_timeZone);
#endif
break;
}
Q_UNREACHABLE();
return 0;
}
// Check the UTC / offsetFromUTC validity
void QDateTimePrivate::checkValidDateTime()
{
switch (m_spec) {
case Qt::OffsetFromUTC:
case Qt::UTC:
// for these, a valid date and a valid time imply a valid QDateTime
if (isValidDate() && isValidTime())
setValidDateTime();
else
clearValidDateTime();
break;
case Qt::TimeZone:
// Defer checking until required as can be expensive
clearValidDateTime();
clearTimeZoneCached();
m_offsetFromUtc = 0;
break;
case Qt::LocalTime:
// Defer checking until required as can be expensive
clearValidDateTime();
m_offsetFromUtc = 0;
// for these, we need to check whether the timezone is valid and whether
// the time is valid in that timezone. Expensive, but no other option.
refreshDateTime();
break;
}
}
@ -2741,12 +2757,6 @@ void QDateTimePrivate::refreshDateTime()
// Always set by setDateTime so just return
return;
case Qt::TimeZone:
// If already cached then don't need to refresh as tz won't change
if (isTimeZoneCached())
return;
// Flag that will have a cached result after calculations
setTimeZoneCached();
break;
case Qt::LocalTime:
break;
}
@ -3082,7 +3092,6 @@ bool QDateTime::isNull() const
bool QDateTime::isValid() const
{
d->refreshDateTime();
return (d->isValidDateTime());
}
@ -3143,13 +3152,11 @@ Qt::TimeSpec QDateTime::timeSpec() const
QTimeZone QDateTime::timeZone() const
{
switch (d->m_spec) {
case Qt::OffsetFromUTC:
if (!d->m_timeZone.isValid())
d->m_timeZone = QTimeZone(d->m_offsetFromUtc);
return d->m_timeZone;
case Qt::UTC:
return QTimeZone::utc();
case Qt::OffsetFromUTC:
case Qt::TimeZone:
Q_ASSERT(d->m_timeZone.isValid());
return d->m_timeZone;
case Qt::LocalTime:
return QTimeZone::systemTimeZone();
@ -3178,7 +3185,6 @@ QTimeZone QDateTime::timeZone() const
int QDateTime::offsetFromUtc() const
{
d->refreshDateTime();
return d->m_offsetFromUtc;
}
@ -3350,7 +3356,7 @@ void QDateTime::setTimeZone(const QTimeZone &toZone)
d->m_spec = Qt::TimeZone;
d->m_offsetFromUtc = 0;
d->m_timeZone = toZone;
d->m_status = d->m_status & ~QDateTimePrivate::ValidDateTime & ~QDateTimePrivate::TimeZoneCached;
d->refreshDateTime();
}
#endif // QT_BOOTSTRAPPED
@ -3371,7 +3377,6 @@ void QDateTime::setTimeZone(const QTimeZone &toZone)
*/
qint64 QDateTime::toMSecsSinceEpoch() const
{
d->refreshDateTime();
return d->toMSecsSinceEpoch();
}
@ -3453,8 +3458,8 @@ void QDateTime::setMSecsSinceEpoch(qint64 msecs)
d->m_status = d->m_status
| QDateTimePrivate::ValidDate
| QDateTimePrivate::ValidTime
| QDateTimePrivate::ValidDateTime
| QDateTimePrivate::TimeZoneCached;
| QDateTimePrivate::ValidDateTime;
d->refreshDateTime();
#endif // QT_BOOTSTRAPPED
break;
case Qt::LocalTime: {

2
src/corelib/tools/qdatetime.h
View File

@ -326,7 +326,7 @@ private:
// ### Qt6: Using a private here has high impact on runtime
// on users such as QFileInfo. In Qt 6, the data members
// should be inlined.
QExplicitlySharedDataPointer<QDateTimePrivate> d;
QSharedDataPointer<QDateTimePrivate> d;
#ifndef QT_NO_DATASTREAM
friend Q_CORE_EXPORT QDataStream &operator<<(QDataStream &, const QDateTime &);

14
src/corelib/tools/qdatetime_p.h
View File

@ -79,10 +79,9 @@ public:
enum StatusFlag {
NullDate = 0x01,
NullTime = 0x02,
ValidDate = 0x04,
ValidTime = 0x08,
ValidDateTime = 0x10,
TimeZoneCached = 0x20,
ValidDate = 0x04, // just the date field
ValidTime = 0x08, // just the time field
ValidDateTime = 0x10, // the whole object (including timezone)
SetToStandardTime = 0x40,
SetToDaylightTime = 0x80
};
@ -120,7 +119,7 @@ public:
DaylightStatus daylightStatus() const;
// Returns msecs since epoch, assumes offset value is current
inline qint64 toMSecsSinceEpoch() const { return (m_msecs - (m_offsetFromUtc * 1000)); }
inline qint64 toMSecsSinceEpoch() const;
void checkValidDateTime();
void refreshDateTime();
@ -133,14 +132,11 @@ public:
inline bool isValidDateTime() const { return m_status & ValidDateTime; }
inline void setValidDateTime() { m_status |= ValidDateTime; }
inline void clearValidDateTime() { m_status &= ~ValidDateTime; }
inline bool isTimeZoneCached() const { return m_status & TimeZoneCached; }
inline void setTimeZoneCached() { m_status |= TimeZoneCached; }
inline void clearTimeZoneCached() { m_status &= ~TimeZoneCached; }
inline void clearSetToDaylightStatus() { m_status &= ~(SetToStandardTime | SetToDaylightTime); }
#ifndef QT_BOOTSTRAPPED
static qint64 zoneMSecsToEpochMSecs(qint64 msecs, const QTimeZone &zone,
QDate *localDate, QTime *localTime);
QDate *localDate = 0, QTime *localTime = 0);
#endif // QT_BOOTSTRAPPED
static inline qint64 minJd() { return QDate::minJd(); }

53
tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp
View File

@ -142,6 +142,9 @@ private slots:
void isDaylightTime() const;
void daylightTransitions() const;
void timeZones() const;
#if defined(Q_OS_UNIX)
void systemTimeZoneChange() const;
#endif
void invalid() const;
@ -2984,6 +2987,56 @@ 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"));
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);
QDateTime tzDate = QDateTime(QDate(2012, 6, 1), QTime(2, 15, 30), QTimeZone("Australia/Brisbane"));
qint64 localMsecs = localDate.toMSecsSinceEpoch();
qint64 utcMsecs = utcDate.toMSecsSinceEpoch();
qint64 tzMsecs = tzDate.toMSecsSinceEpoch();
// check that Australia/Brisbane is known
QVERIFY(tzDate.timeZone().isValid());
// Change to Indian time
setTimeZone(QByteArray("IST-05:30"));
QCOMPARE(localDate, QDateTime(QDate(2012, 6, 1), QTime(2, 15, 30), Qt::LocalTime));
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
{
QDateTime invalidDate = QDateTime(QDate(0, 0, 0), QTime(-1, -1, -1));