From 10324df60ebf42012adb81e0cdb9122df2aac72a Mon Sep 17 00:00:00 2001
From: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
Date: Sun, 24 Sep 2023 22:39:13 +0200
Subject: [PATCH] QWeakPointer: optimize the converting constructor

The converting constructor of QWeakPointer<T> from a QWeakPointer<X>
needs to adjust the X* received by the "source" to a T*.

In case of non-virtual inheritance, this adjustment can be done
statically, by applying an offset to the pointer. In case of virtual
inheritance, we instead need to dereference the pointer (=access the
pointee), get its vtable, and from there find where (=the offset at
which) the T subobject is located.

This latter scenario requires the pointee to be alive throughout this
operation. Since QWeakPointer isn't an owning smart pointer, it's
perfectly possible that the pointee has already been deleted (the
"source" QWeakPointer<X> is dangling) or is being deleted (e.g. from
another thread that has just released the last QSharedPointer). For
this reason the converting constructor of QWeakPointer employs a
protection: it will lock() itself, and extract the raw pointer from the
QSharedPointer so obtained. This ensures that we won't access a dangling
pointer or a pointee about to be deleted.

We can however limit this (relatively expensive) protection only to the
case where there is virtual inheritance. In the other cases we don't
need it. This commit overloads the converting constructor for
QWeakPointer to deal with the two scenarios separately, and only lock()
in case of virtual inheritance.

Change-Id: I7194b90478cf35024e60cff542091308e4131ec2
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
---
 src/corelib/tools/qsharedpointer_impl.h | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h
index a9c6babbf58..5dfc4614f91 100644
--- a/src/corelib/tools/qsharedpointer_impl.h
+++ b/src/corelib/tools/qsharedpointer_impl.h
@@ -28,6 +28,7 @@ QT_END_NAMESPACE
 #include <QtCore/qatomic.h>
 #include <QtCore/qhashfunctions.h>
 #include <QtCore/qmetatype.h> // for IsPointerToTypeDerivedFromQObject
+#include <QtCore/qxptype_traits.h>
 
 #include <memory>
 
@@ -537,6 +538,12 @@ class QWeakPointer
     template <typename X>
     using IfCompatible = typename std::enable_if<std::is_convertible<X*, T*>::value, bool>::type;
 
+    template <typename X>
+    using IfVirtualBase = typename std::enable_if<qxp::is_virtual_base_of_v<T, X>, bool>::type;
+
+    template <typename X>
+    using IfNotVirtualBase = typename std::enable_if<!qxp::is_virtual_base_of_v<T, X>, bool>::type;
+
 public:
     typedef T element_type;
     typedef T value_type;
@@ -566,7 +573,15 @@ public:
     }
     QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_MOVE_AND_SWAP(QWeakPointer)
 
-    template <class X, IfCompatible<X> = true>
+    template <class X, IfCompatible<X> = true, IfNotVirtualBase<X> = true>
+    Q_NODISCARD_CTOR
+    QWeakPointer(QWeakPointer<X> &&other) noexcept
+        : d(std::exchange(other.d, nullptr)),
+          value(std::exchange(other.value, nullptr))
+    {
+    }
+
+    template <class X, IfCompatible<X> = true, IfVirtualBase<X> = true>
     Q_NODISCARD_CTOR
     QWeakPointer(QWeakPointer<X> &&other) noexcept
         : d(other.d), value(other.toStrongRef().get()) // must go through QSharedPointer, see below