QMutex: make sure we try_lock_for no shorter than the duration passed

By templating on the <chrono> types and unconditionally using
duration_cast to coerce the duration into a milliseconds, we
allowed code such as

   mutex.try_lock_for(10us)

to compile, which is misleading, since it's actually a zero-
timeout try_lock().

Feedback from the std-discussions mailing list is that the
wait_for functions should wait for _at least_ the duration
given, because that is the natural direction of variance
(tasks becoming ready to run might not get a CPU immediately,
causing delays), while an interface that documents to wait
_no more_ than the given duration is promising something it
cannot fulfill.

Fix by converting the given duration to the smallest number
of milliseconds not less than the original duration. If that
is not representable in an int, use INT_MAX, emulating the
effect of a spurious wakeup, which are allowed to happen if
the function returns false in that case.

In the above example, the try_lock_for call is now equivalent
to

  mutex.tryLock(1);

The tryLock() docs state that the actual waiting time does
not exceed the given milliseconds, but fixing that is a
separate issue.

Change-Id: Id4cbbea0ecc6fd2f94bb5aef28a1658be3728e52
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

1 files changed, 114 insertions, 0 deletions

diff --git a/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp b/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp
index b24ecfcd43..bf778e9fd1 100644
--- a/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp
+++ b/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp
@@ -44,8 +44,19 @@
 class tst_QMutex : public QObject
 {
     Q_OBJECT
+public:
+    enum class TimeUnit {
+        Nanoseconds,
+        Microseconds,
+        Milliseconds,
+        Seconds,
+    };
+    Q_ENUM(TimeUnit);
+
 private slots:
     void initTestCase();
+    void convertToMilliseconds_data();
+    void convertToMilliseconds();
     void tryLock_non_recursive();
     void try_lock_for_non_recursive();
     void try_lock_until_non_recursive();
@@ -122,6 +133,109 @@ void tst_QMutex::initTestCase()
     initializeSystemTimersResolution();
 }
 
+void tst_QMutex::convertToMilliseconds_data()
+{
+    QTest::addColumn<TimeUnit>("unit");
+    QTest::addColumn<double>("doubleValue");
+    QTest::addColumn<qint64>("intValue");
+    QTest::addColumn<qint64>("expected");
+
+
+    auto add = [](TimeUnit unit, double d, long long i, qint64 expected) {
+        const QScopedArrayPointer<char> enumName(QTest::toString(unit));
+        QTest::newRow(qPrintable(QString::asprintf("%s:%f:%lld", enumName.data(), d, i)))
+            << unit << d << qint64(i) << expected;
+    };
+
+    auto forAllUnitsAdd = [=](double d, long long i, qint64 expected) {
+        for (auto unit : {TimeUnit::Nanoseconds, TimeUnit::Microseconds, TimeUnit::Milliseconds, TimeUnit::Seconds})
+            add(unit, d, i, expected);
+    };
+
+    forAllUnitsAdd(-0.5, -1, 0);     // all negative values result in 0
+
+    forAllUnitsAdd(0, 0, 0);
+
+    add(TimeUnit::Nanoseconds,                 1,               1, 1);
+    add(TimeUnit::Nanoseconds, 1000 * 1000,       1000 * 1000,     1);
+    add(TimeUnit::Nanoseconds, 1000 * 1000 + 0.5, 1000 * 1000 + 1, 2);
+
+    add(TimeUnit::Microseconds,          1,        1, 1);
+    add(TimeUnit::Microseconds, 1000,       1000,     1);
+    add(TimeUnit::Microseconds, 1000 + 0.5, 1000 + 1, 2);
+
+    add(TimeUnit::Milliseconds, 1,   1, 1);
+    add(TimeUnit::Milliseconds, 1.5, 2, 2);
+
+    add(TimeUnit::Seconds, 0.9991, 1, 1000);
+
+    //
+    // overflowing int results in INT_MAX (equivalent to a spurious wakeup after ~24 days); check it:
+    //
+
+    // spot on:
+    add(TimeUnit::Nanoseconds,  INT_MAX * 1000. * 1000, INT_MAX * Q_INT64_C(1000) * 1000, INT_MAX);
+    add(TimeUnit::Microseconds, INT_MAX * 1000.,        INT_MAX * Q_INT64_C(1000),        INT_MAX);
+    add(TimeUnit::Milliseconds, INT_MAX,                INT_MAX,                          INT_MAX);
+
+    // minimally above:
+    add(TimeUnit::Nanoseconds,  INT_MAX * 1000. * 1000 + 1, INT_MAX * Q_INT64_C(1000) * 1000 + 1, INT_MAX);
+    add(TimeUnit::Microseconds, INT_MAX * 1000.        + 1, INT_MAX * Q_INT64_C(1000)        + 1, INT_MAX);
+    add(TimeUnit::Milliseconds, INT_MAX               + 1., INT_MAX               + Q_INT64_C(1), INT_MAX);
+    add(TimeUnit::Seconds,      INT_MAX / 1000.        + 1, INT_MAX / 1000                   + 1, INT_MAX);
+
+    // minimally below:
+    add(TimeUnit::Nanoseconds,  INT_MAX * 1000. * 1000 - 1, INT_MAX * Q_INT64_C(1000) * 1000 - 1, INT_MAX);
+    add(TimeUnit::Microseconds, INT_MAX * 1000.        - 1, INT_MAX * Q_INT64_C(1000)        - 1, INT_MAX);
+    add(TimeUnit::Milliseconds, INT_MAX              - 0.1, INT_MAX                             , INT_MAX);
+
+}
+
+void tst_QMutex::convertToMilliseconds()
+{
+#if !QT_HAS_INCLUDE(<chrono>)
+    QSKIP("This test requires <chrono>");
+#else
+    QFETCH(TimeUnit, unit);
+    QFETCH(double, doubleValue);
+    QFETCH(qint64, intValue);
+    QFETCH(qint64, expected);
+
+    Q_CONSTEXPR qint64 maxShort = std::numeric_limits<short>::max();
+    Q_CONSTEXPR qint64 maxInt = std::numeric_limits<int>::max();
+    Q_CONSTEXPR qint64 maxUInt = std::numeric_limits<uint>::max();
+
+    switch (unit) {
+#define CASE(Unit, Period) \
+    case TimeUnit::Unit: \
+        DO(double,  Period, doubleValue); \
+        if (intValue < maxShort) \
+            DO(short,   Period, short(intValue)); \
+        if (intValue < maxInt) \
+            DO(int,     Period, int(intValue)); \
+        DO(qint64,  Period, intValue); \
+        if (intValue >= 0) { \
+            if (intValue < maxUInt) \
+                DO(uint,    Period, uint(intValue)); \
+            DO(quint64, Period, quint64(intValue)); \
+        } \
+        break
+#define DO(Rep, Period, val) \
+    do { \
+        const std::chrono::duration<Rep, Period> wait((val)); \
+        QCOMPARE(QMutex::convertToMilliseconds(wait), expected); \
+    } while (0)
+
+    CASE(Nanoseconds,  std::nano);
+    CASE(Microseconds, std::micro);
+    CASE(Milliseconds, std::milli);
+    CASE(Seconds,      std::ratio<1>);
+#undef DO
+#undef CASE
+    }
+#endif
+}
+
 void tst_QMutex::tryLock_non_recursive()
 {
     class Thread : public QThread