QMutex: add QDeadlineTimer-based tryLocks

This simplifies the code greatly, because we don't need to use
QtPrivate::convertToMilliseconds any more, as QDeadlineTimer has
nanosecond precision.

Internally it becomes simpler too because lockInternal was already using
QDeadlineTimer. I just had to use the parameter instead and update the
two non-futex implementations to take it again. This may even be fixing
a mistake in case sem_timedwait(2) got interrupted.

Change-Id: I6f518d59e63249ddbf43fffd1759fed9f50b3606
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/corelib/compat/removed_api.cpp

@@ -603,6 +603,7 @@ QStringView QXmlStreamAttributes::value(QLatin1StringView qualifiedName) const
 
 // inlined API
 #if QT_CONFIG(thread)
+#include "qmutex.h"
 #include "qreadwritelock.h"
 #endif
 

src/corelib/thread/qmutex.cpp

@@ -146,6 +146,23 @@ void QBasicMutex::destroyInternal(QMutexPrivate *d)
     \sa lock(), unlock()
 */
 
+/*! \fn bool QMutex::tryLock(QDeadlineTimer timer)
+    \since 6.6
+
+    Attempts to lock the mutex. This function returns \c true if the lock
+    was obtained; otherwise it returns \c false. If another thread has
+    locked the mutex, this function will wait until \a timer expires
+    for the mutex to become available.
+
+    If the lock was obtained, the mutex must be unlocked with unlock()
+    before another thread can successfully lock it.
+
+    Calling this function multiple times on the same mutex from the
+    same thread will cause a \e dead-lock.
+
+    \sa lock(), unlock()
+*/
+
 /*! \fn bool QMutex::tryLock()
     \overload
 
@@ -279,6 +296,8 @@ QRecursiveMutex::~QRecursiveMutex()
 */
 
 /*!
+    \fn QRecursiveMutex::tryLock(int timeout)
+
     Attempts to lock the mutex. This function returns \c true if the lock
     was obtained; otherwise it returns \c false. If another thread has
     locked the mutex, this function will wait for at most \a timeout
@@ -296,7 +315,24 @@ QRecursiveMutex::~QRecursiveMutex()
 
     \sa lock(), unlock()
 */
-bool QRecursiveMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
+
+/*!
+    \since 6.6
+
+    Attempts to lock the mutex. This function returns \c true if the lock
+    was obtained; otherwise it returns \c false. If another thread has
+    locked the mutex, this function will wait until \a timeout expires
+    for the mutex to become available.
+
+    If the lock was obtained, the mutex must be unlocked with unlock()
+    before another thread can successfully lock it.
+
+    Calling this function multiple times on the same mutex from the
+    same thread is allowed.
+
+    \sa lock(), unlock()
+*/
+bool QRecursiveMutex::tryLock(QDeadlineTimer timeout) QT_MUTEX_LOCK_NOEXCEPT
 {
     unsigned tsanFlags = QtTsan::MutexWriteReentrant | QtTsan::TryLock;
     QtTsan::mutexPreLock(this, tsanFlags);
@@ -309,7 +345,7 @@ bool QRecursiveMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
         return true;
     }
     bool success = true;
-    if (timeout == -1) {
+    if (timeout.isForever()) {
         mutex.lock();
     } else {
         success = mutex.tryLock(timeout);
@@ -622,25 +658,37 @@ void QBasicMutex::lockInternal() QT_MUTEX_LOCK_NOEXCEPT
 /*!
     \internal helper for lock(int)
  */
+#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0)
 bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 {
     if (timeout == 0)
         return false;
 
+    return lockInternal(QDeadlineTimer(timeout));
+}
+#endif
+
+/*!
+    \internal helper for tryLock(QDeadlineTimer)
+ */
+bool QBasicMutex::lockInternal(QDeadlineTimer deadlineTimer) QT_MUTEX_LOCK_NOEXCEPT
+{
+    qint64 remainingTime = deadlineTimer.remainingTimeNSecs();
+    if (remainingTime == 0)
+        return false;
+
     if (futexAvailable()) {
-        if (Q_UNLIKELY(timeout < 0)) {
+        if (Q_UNLIKELY(remainingTime < 0)) {    // deadlineTimer.isForever()
             lockInternal();
             return true;
         }
 
-        QDeadlineTimer deadlineTimer(timeout);
         // The mutex is already locked, set a bit indicating we're waiting.
         // Note we must set to dummyFutexValue because there could be other threads
         // also waiting.
         if (d_ptr.fetchAndStoreAcquire(dummyFutexValue()) == nullptr)
             return true;
 
-        qint64 remainingTime = deadlineTimer.remainingTimeNSecs();
         Q_FOREVER {
             if (!futexWait(d_ptr, dummyFutexValue(), remainingTime))
                 return false;
@@ -665,7 +713,7 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT
             continue;
 
         if (copy == dummyLocked()) {
-            if (timeout == 0)
+            if (remainingTime == 0)
                 return false;
             // The mutex is locked but does not have a QMutexPrivate yet.
             // we need to allocate a QMutexPrivate
@@ -680,7 +728,7 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT
         }
 
         QMutexPrivate *d = static_cast<QMutexPrivate *>(copy);
-        if (timeout == 0 && !d->possiblyUnlocked.loadRelaxed())
+        if (remainingTime == 0 && !d->possiblyUnlocked.loadRelaxed())
             return false;
 
         // At this point we have a pointer to a QMutexPrivate. But the other thread
@@ -733,7 +781,7 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT
             continue;
         }
 
-        if (d->wait(timeout)) {
+        if (d->wait(deadlineTimer)) {
             // reset the possiblyUnlocked flag if needed (and deref its corresponding reference)
             if (d->possiblyUnlocked.loadRelaxed() && d->possiblyUnlocked.testAndSetRelaxed(true, false))
                 d->deref();
@@ -742,8 +790,8 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT
             Q_ASSERT(d == d_ptr.loadRelaxed());
             return true;
         } else {
-            Q_ASSERT(timeout >= 0);
+            Q_ASSERT(remainingTime >= 0);
-            //timeout
+            // timed out
             d->derefWaiters(1);
             //There may be a race in which the mutex is unlocked right after we timed out,
             // and before we deref the waiters, so maybe the mutex is actually unlocked.

src/corelib/thread/qmutex.h

@@ -6,6 +6,7 @@
 
 #include <QtCore/qglobal.h>
 #include <QtCore/qatomic.h>
+#include <QtCore/qdeadlinetimer.h>
 #include <QtCore/qtsan_impl.h>
 #include <new>
 
@@ -110,7 +111,10 @@ private:
     }
 
     void lockInternal() QT_MUTEX_LOCK_NOEXCEPT;
+    bool lockInternal(QDeadlineTimer timeout) QT_MUTEX_LOCK_NOEXCEPT;
+#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0)
     bool lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT;
+#endif
     void unlockInternal() noexcept;
     void destroyInternal(QMutexPrivate *d);
 
@@ -146,6 +150,11 @@ public:
 
     using QBasicMutex::tryLock;
     bool tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
+    {
+        return tryLock(QDeadlineTimer(timeout));
+    }
+
+    bool tryLock(QDeadlineTimer timeout) QT_MUTEX_LOCK_NOEXCEPT
     {
         unsigned tsanFlags = QtTsan::TryLock;
         QtTsan::mutexPreLock(this, tsanFlags);
@@ -166,21 +175,24 @@ public:
         return success;
     }
 
+#ifndef Q_QDOC
+    // because tryLock(QDeadlineTimer::Forever) is tryLock(0)
+    bool tryLock(QDeadlineTimer::ForeverConstant) QT_MUTEX_LOCK_NOEXCEPT
+    { lock(); return true; }
+#endif
+
     // TimedLockable concept
     template <class Rep, class Period>
     bool try_lock_for(std::chrono::duration<Rep, Period> duration)
     {
-        return tryLock(QtPrivate::convertToMilliseconds(duration));
+        return tryLock(QDeadlineTimer(duration));
     }
 
     // TimedLockable concept
     template<class Clock, class Duration>
     bool try_lock_until(std::chrono::time_point<Clock, Duration> timePoint)
     {
-        // Implemented in terms of try_lock_for to honor the similar
+        return tryLock(QDeadlineTimer(timePoint));
-        // requirement in N4606 § 30.4.1.3 [thread.timedmutex.requirements]/12.
-
-        return try_lock_for(timePoint - Clock::now());
     }
 };
 
@@ -201,8 +213,10 @@ public:
 
     // BasicLockable concept
     void lock() QT_MUTEX_LOCK_NOEXCEPT
-    { tryLock(-1); }
+    { tryLock(QDeadlineTimer(QDeadlineTimer::Forever)); }
+    QT_CORE_INLINE_SINCE(6, 6)
     bool tryLock(int timeout = 0) QT_MUTEX_LOCK_NOEXCEPT;
+    bool tryLock(QDeadlineTimer timer) QT_MUTEX_LOCK_NOEXCEPT;
     // BasicLockable concept
     void unlock() noexcept;
 
@@ -213,20 +227,30 @@ public:
     template <class Rep, class Period>
     bool try_lock_for(std::chrono::duration<Rep, Period> duration)
     {
-        return tryLock(QtPrivate::convertToMilliseconds(duration));
+        return tryLock(QDeadlineTimer(duration));
     }
 
     // TimedLockable concept
     template<class Clock, class Duration>
     bool try_lock_until(std::chrono::time_point<Clock, Duration> timePoint)
     {
-        // Implemented in terms of try_lock_for to honor the similar
+        return tryLock(QDeadlineTimer(timePoint));
-        // requirement in N4606 § 30.4.1.3 [thread.timedmutex.requirements]/12.
-
-        return try_lock_for(timePoint - Clock::now());
     }
+
+#ifndef Q_QDOC
+    // because tryLock(QDeadlineTimer::Forever) is tryLock(0)
+    bool tryLock(QDeadlineTimer::ForeverConstant) QT_MUTEX_LOCK_NOEXCEPT
+    { lock(); return true; }
+#endif
 };
 
+#if QT_CORE_INLINE_IMPL_SINCE(6, 6)
+bool QRecursiveMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
+{
+    return tryLock(QDeadlineTimer(timeout));
+}
+#endif
+
 template <typename Mutex>
 class [[nodiscard]] QMutexLocker
 {

src/corelib/thread/qmutex_mac.cpp

@@ -5,6 +5,8 @@
 #include "qmutex.h"
 #include "qmutex_p.h"
 
+#include "private/qcore_unix_p.h"
+
 #include <mach/mach.h>
 #include <mach/task.h>
 
@@ -26,18 +28,19 @@ QMutexPrivate::~QMutexPrivate()
         qWarning("QMutex: failed to destroy semaphore, error %d", r);
 }
 
-bool QMutexPrivate::wait(int timeout)
+bool QMutexPrivate::wait(QDeadlineTimer timeout)
 {
     kern_return_t r;
-    if (timeout < 0) {
+    if (timeout.isForever()) {
         do {
             r = semaphore_wait(mach_semaphore);
         } while (r == KERN_ABORTED);
         Q_ASSERT(r == KERN_SUCCESS);
     } else {
+        timespec tv = durationToTimespec(timeout.remainingTimeAsDuration());
         mach_timespec_t ts;
-        ts.tv_nsec = ((timeout % 1000) * 1000) * 1000;
+        ts.tv_nsec = tv.tv_nsec;
-        ts.tv_sec = (timeout / 1000);
+        ts.tv_sec = tv.tv_sec;
         r = semaphore_timedwait(mach_semaphore, ts);
     }
     return (r == KERN_SUCCESS);

src/corelib/thread/qmutex_p.h

@@ -43,7 +43,7 @@ public:
     ~QMutexPrivate();
     QMutexPrivate();
 
-    bool wait(int timeout = -1);
+    bool wait(QDeadlineTimer timeout = QDeadlineTimer::Forever);
     void wakeUp() noexcept;
 
     // Control the lifetime of the privates

src/corelib/thread/qmutex_unix.cpp

@@ -36,21 +36,18 @@ QMutexPrivate::~QMutexPrivate()
     report_error(sem_destroy(&semaphore), "QMutex", "sem_destroy");
 }
 
-bool QMutexPrivate::wait(int timeout)
+bool QMutexPrivate::wait(QDeadlineTimer timeout)
 {
     int errorCode;
-    if (timeout < 0) {
+    if (timeout.isForever()) {
         do {
             errorCode = sem_wait(&semaphore);
         } while (errorCode && errno == EINTR);
         report_error(errorCode, "QMutex::lock()", "sem_wait");
     } else {
-        timespec ts;
-        report_error(clock_gettime(CLOCK_REALTIME, &ts), "QMutex::lock()", "clock_gettime");
-        ts.tv_sec += timeout / 1000;
-        ts.tv_nsec += timeout % 1000 * Q_UINT64_C(1000) * 1000;
-        normalizedTimespec(ts);
         do {
+            auto tp = timeout.deadline<std::chrono::system_clock>();
+            timespec ts = durationToTimespec(tp.time_since_epoch());
             errorCode = sem_timedwait(&semaphore, &ts);
         } while (errorCode && errno == EINTR);