From ecfda98d1f91c6a7da0d89826558d856cd88e670 Mon Sep 17 00:00:00 2001
From: Allan Sandfeld Jensen <allan.jensen@qt.io>
Date: Thu, 1 Oct 2020 10:40:47 +0200
Subject: Remove unnecessary ref-counting of QRunnable

It could never be higher than 1 anyway.

Change-Id: If33c7978a4397a08e9eb091926726725d8bd3ea6
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>
---
 src/corelib/thread/qrunnable.h     | 11 ++++-------
 src/corelib/thread/qthreadpool.cpp | 28 +++-------------------------
 2 files changed, 7 insertions(+), 32 deletions(-)

(limited to 'src/corelib/thread')

diff --git a/src/corelib/thread/qrunnable.h b/src/corelib/thread/qrunnable.h
index 9f8ab2a3d0..22d508d35c 100644
--- a/src/corelib/thread/qrunnable.h
+++ b/src/corelib/thread/qrunnable.h
@@ -47,21 +47,18 @@ QT_BEGIN_NAMESPACE
 
 class Q_CORE_EXPORT QRunnable
 {
-    int ref; // Qt6: Make this a bool, or make autoDelete() virtual.
+    bool m_autoDelete = true;
 
-    friend class QThreadPool;
-    friend class QThreadPoolPrivate;
-    friend class QThreadPoolThread;
     Q_DISABLE_COPY(QRunnable)
 public:
     virtual void run() = 0;
 
-    QRunnable() : ref(0) { }
+    constexpr QRunnable() noexcept = default;
     virtual ~QRunnable();
     static QRunnable *create(std::function<void()> functionToRun);
 
-    bool autoDelete() const { return ref != -1; }
-    void setAutoDelete(bool _autoDelete) { ref = _autoDelete ? 0 : -1; }
+    bool autoDelete() const { return m_autoDelete; }
+    void setAutoDelete(bool autoDelete) { m_autoDelete = autoDelete; }
 };
 
 QT_END_NAMESPACE
diff --git a/src/corelib/thread/qthreadpool.cpp b/src/corelib/thread/qthreadpool.cpp
index 5765bf6b09..1c1bf38f9b 100644
--- a/src/corelib/thread/qthreadpool.cpp
+++ b/src/corelib/thread/qthreadpool.cpp
@@ -88,9 +88,8 @@ void QThreadPoolThread::run()
 
         do {
             if (r) {
+                // If autoDelete() is false, r might already be deleted after run(), so check status now.
                 const bool del = r->autoDelete();
-                Q_ASSERT(!del || r->ref == 1);
-
 
                 // run the task
                 locker.unlock();
@@ -330,7 +329,6 @@ void QThreadPoolPrivate::clear()
         while (!page->isFinished()) {
             QRunnable *r = page->pop();
             if (r && r->autoDelete()) {
-                Q_ASSERT(r->ref == 1);
                 locker.unlock();
                 delete r;
                 locker.relock();
@@ -371,10 +369,6 @@ bool QThreadPool::tryTake(QRunnable *runnable)
                 d->queue.removeOne(page);
                 delete page;
             }
-            if (runnable->autoDelete()) {
-                Q_ASSERT(runnable->ref == 1);
-                --runnable->ref; // undo ++ref in start()
-            }
             return true;
         }
     }
@@ -393,14 +387,13 @@ void QThreadPoolPrivate::stealAndRunRunnable(QRunnable *runnable)
     Q_Q(QThreadPool);
     if (!q->tryTake(runnable))
         return;
+    // If autoDelete() is false, runnable might already be deleted after run(), so check status now.
     const bool del = runnable->autoDelete();
 
     runnable->run();
 
-    if (del) {
-        Q_ASSERT(runnable->ref == 0); // tryTake already deref'ed
+    if (del)
         delete runnable;
-    }
 }
 
 /*!
@@ -508,10 +501,6 @@ void QThreadPool::start(QRunnable *runnable, int priority)
 
     Q_D(QThreadPool);
     QMutexLocker locker(&d->mutex);
-    if (runnable->autoDelete()) {
-        Q_ASSERT(runnable->ref == 0);
-        ++runnable->ref;
-    }
 
     if (!d->tryStart(runnable)) {
         d->enqueueTask(runnable, priority);
@@ -558,22 +547,11 @@ bool QThreadPool::tryStart(QRunnable *runnable)
     if (!runnable)
         return false;
 
-    if (runnable->autoDelete()) {
-        Q_ASSERT(runnable->ref == 0);
-        ++runnable->ref;
-    }
-
     Q_D(QThreadPool);
     QMutexLocker locker(&d->mutex);
     if (d->tryStart(runnable))
         return true;
 
-    // Undo the reference above as we did not start the runnable and
-    // take over ownership.
-    if (runnable->autoDelete()) {
-        --runnable->ref;
-        Q_ASSERT(runnable->ref == 0);
-    }
     return false;
 }
 
-- 
cgit v1.2.3