QSharedPointer: do allow calling deleters on null pointers

I don't know why std::shared_ptr allows this, but why not. [ChangeLog][Important Behavior Changes] QSharedPointer objects will now call custom deleters even when the pointer being tracked was null. This behavior is the same as std::shared_ptr. Fixes: QTBUG-85285 Change-Id: I24006db8360041f598c5fffd161c260df0313b55 Reviewed-by: Giuseppe D'Angelo <[email protected]> (cherry picked from commit 95afe6b) Reviewed-by: Qt Cherry-pick Bot <[email protected]>
boris-fx · Jul 31, 2020 · 73d6c20 · 73d6c20
1 parent 18df9b7
commit 73d6c20
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 13 deletions.
diff --git a/src/corelib/tools/qsharedpointer.cpp b/src/corelib/tools/qsharedpointer.cpp
@@ -1,7 +1,8 @@
 /****************************************************************************
 **
-** Copyright (C) 2016 The Qt Company Ltd.
-** Copyright (C) 2016 Intel Corporation.
+** Copyright (C) 2020 The Qt Company Ltd.
+** Copyright (C) 2020 Intel Corporation.
+** Copyright (C) 2019 Klarälvdalens Datakonsult AB.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtCore module of the Qt Toolkit.
@@ -1562,6 +1563,12 @@ void QtSharedPointer::internalSafetyCheckAdd(const void *d_ptr, const volatile v
     if (!kp)
         return;                 // end-game: the application is being destroyed already
 
+    if (!ptr) {
+        // nullptr is allowed to be tracked by more than one QSharedPointer, so we
+        // need something else to put in our tracking structures
+        ptr = d_ptr;
+    }
+
     QMutexLocker lock(&kp->mutex);
     Q_ASSERT(!kp->dPointers.contains(d_ptr));
 

diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h
@@ -1,7 +1,8 @@
 /****************************************************************************
 **
-** Copyright (C) 2016 The Qt Company Ltd.
-** Copyright (C) 2016 Intel Corporation.
+** Copyright (C) 2019 The Qt Company Ltd.
+** Copyright (C) 2020 Intel Corporation.
+** Copyright (C) 2019 Klarälvdalens Datakonsult AB.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtCore module of the Qt Toolkit.
@@ -107,7 +108,7 @@ namespace QtSharedPointer {
 
     template <class T, typename Klass, typename RetVal>
     inline void executeDeleter(T *t, RetVal (Klass:: *memberDeleter)())
-    { (t->*memberDeleter)(); }
+    { if (t) (t->*memberDeleter)(); }
     template <class T, typename Deleter>
     inline void executeDeleter(T *t, Deleter d)
     { d(t); }
@@ -319,7 +320,8 @@ template <class T> class QSharedPointer
     { internalConstruct(ptr, deleter); }
 
     template <typename Deleter>
-    QSharedPointer(std::nullptr_t, Deleter) : value(nullptr), d(nullptr) { }
+    QSharedPointer(std::nullptr_t, Deleter deleter) : value(nullptr)
+    { internalConstruct(static_cast<T *>(nullptr), deleter); }
 
     QSharedPointer(const QSharedPointer &other) noexcept : value(other.value), d(other.d)
     { if (d) ref(); }
@@ -470,11 +472,6 @@ template <class T> class QSharedPointer
     template <typename X, typename Deleter>
     inline void internalConstruct(X *ptr, Deleter deleter)
     {
-        if (!ptr) {
-            d = nullptr;
-            return;
-        }
-
         typedef QtSharedPointer::ExternalRefCountWithCustomDeleter<X, Deleter> Private;
 # ifdef QT_SHAREDPOINTER_TRACK_POINTERS
         typename Private::DestroyerFn actualDeleter = &Private::safetyCheckDeleter;

diff --git a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp
@@ -1,7 +1,8 @@
 /****************************************************************************
 **
-** Copyright (C) 2016 The Qt Company Ltd.
-** Copyright (C) 2016 Intel Corporation.
+** Copyright (C) 2019 The Qt Company Ltd.
+** Copyright (C) 2020 Intel Corporation.
+** Copyright (C) 2019 Klarälvdalens Datakonsult AB.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the test suite of the Qt Toolkit.
@@ -95,6 +96,7 @@ private slots:
     void constCorrectness();
     void customDeleter();
     void lambdaCustomDeleter();
+    void customDeleterOnNullptr();
     void creating();
     void creatingCvQualified();
     void creatingVariadic();
@@ -1846,6 +1848,33 @@ void tst_QSharedPointer::lambdaCustomDeleter()
     safetyCheck();
 }
 
+void tst_QSharedPointer::customDeleterOnNullptr()
+{
+    Data *null = nullptr;
+    int callCount = 0;
+    auto deleter = [&callCount](Data *) { ++callCount; };
+    {
+        QSharedPointer<Data> ptr(null, deleter);
+    }
+    safetyCheck();
+    QCOMPARE(callCount, 1);
+
+    callCount = 0;
+    {
+        QSharedPointer<Data> ptr(nullptr, deleter);
+    }
+    safetyCheck();
+    QCOMPARE(callCount, 1);
+
+    callCount = 0;
+    {
+        QSharedPointer<Data> ptr1(null, deleter);
+        QSharedPointer<Data> ptr2(nullptr, deleter);
+    }
+    safetyCheck();
+    QCOMPARE(callCount, 2);
+}
+
 void customQObjectDeleterFn(QObject *obj)
 {
     ++customDeleterFnCallCount;