From a094bf5a893c3cccffff10c1420bfbe3a3c02a7c Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Thu, 3 Jan 2013 15:17:21 -0200 Subject: [PATCH] Don't increase the reference count if dynamic_cast failed If the dynamic_cast failed in QSharedPointer::dynamicCast or qSharedPointerDynamicCast, we should avoid creating the QSharedPointer that shares the weak and strong reference counts. In Qt 5, this does not imply a leak since the original pointer is stored internally for deletion. In Qt 4 it implies a leak under certain circumstances, which this change fixes. Task-number: QTBUG-28924 Change-Id: Id2de140de4cf676461e14b201ad250c53666b79d Reviewed-by: Olivier Goffart --- src/corelib/tools/qsharedpointer_impl.h | 2 + .../tools/qsharedpointer/nontracked.cpp | 116 ++++++++++++++++++ .../corelib/tools/qsharedpointer/nontracked.h | 49 ++++++++ .../tools/qsharedpointer/qsharedpointer.pro | 2 + .../qsharedpointer/tst_qsharedpointer.cpp | 7 ++ 5 files changed, 176 insertions(+) create mode 100644 tests/auto/corelib/tools/qsharedpointer/nontracked.cpp create mode 100644 tests/auto/corelib/tools/qsharedpointer/nontracked.h diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h index 6f3e577e55..cce48e0d07 100644 --- a/src/corelib/tools/qsharedpointer_impl.h +++ b/src/corelib/tools/qsharedpointer_impl.h @@ -786,6 +786,8 @@ template Q_INLINE_TEMPLATE QSharedPointer qSharedPointerDynamicCast(const QSharedPointer &src) { register X *ptr = dynamic_cast(src.data()); // if you get an error in this line, the cast is invalid + if (!ptr) + return QSharedPointer(); return QtSharedPointer::copyAndSetPointer(ptr, src); } template diff --git a/tests/auto/corelib/tools/qsharedpointer/nontracked.cpp b/tests/auto/corelib/tools/qsharedpointer/nontracked.cpp new file mode 100644 index 0000000000..86f2f7a330 --- /dev/null +++ b/tests/auto/corelib/tools/qsharedpointer/nontracked.cpp @@ -0,0 +1,116 @@ +/**************************************************************************** +** +** Copyright (C) 2013 Intel Corporation. +** Contact: http://www.qt-project.org/legal +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and Digia. For licensing terms and +** conditions see http://qt.digia.com/licensing. For further information +** use the contact form at http://qt.digia.com/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 2.1 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 2.1 requirements +** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Digia gives you certain additional +** rights. These rights are described in the Digia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3.0 as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU General Public License version 3.0 requirements will be +** met: http://www.gnu.org/copyleft/gpl.html. +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +/* + * This file exists because tst_qsharedpointer.cpp is compiled with + * QT_SHAREDPOINTER_TRACK_POINTERS. That changes some behavior. + * + * Note that most of these tests may yield false-positives in debug mode, but + * they should not yield false negatives. That is, they may report PASS when + * they are failing, but they should not produce FAILs. + * + * The reason for that is because of C++'s One Definition Rule: the macro + * changes some functions and, in debug mode, they will not be inlined. At link + * time, the two functions would be merged. + */ + +#include +#include + +#include "nontracked.h" + +// We can't name our classes Data and DerivedData: those are in tst_qsharedpointer.cpp +namespace NonTracked { + +class Data +{ +public: + static int destructorCounter; + static int generationCounter; + int generation; + + Data() : generation(++generationCounter) + { } + + virtual ~Data() + { + if (generation <= 0) + qFatal("tst_qsharedpointer: Double deletion!"); + generation = 0; + ++destructorCounter; + } +}; +int Data::generationCounter = 0; +int Data::destructorCounter = 0; + +class DerivedData: public Data +{ +public: + static int derivedDestructorCounter; + int moreData; + DerivedData() : moreData(0) { } + ~DerivedData() { ++derivedDestructorCounter; } +}; +int DerivedData::derivedDestructorCounter = 0; + + +#ifndef QTEST_NO_RTTI +void dynamicCastFailureNoLeak() +{ + Data::destructorCounter = DerivedData::derivedDestructorCounter = 0; + + // see QTBUG-28924 + QSharedPointer a(new Data); + QSharedPointer b = a.dynamicCast(); + QVERIFY(!a.isNull()); + QVERIFY(b.isNull()); + + a.clear(); + b.clear(); + QVERIFY(a.isNull()); + + // verify that the destructors were called + QCOMPARE(Data::destructorCounter, 1); + QCOMPARE(DerivedData::derivedDestructorCounter, 0); +} + +#endif +} // namespace NonTracked diff --git a/tests/auto/corelib/tools/qsharedpointer/nontracked.h b/tests/auto/corelib/tools/qsharedpointer/nontracked.h new file mode 100644 index 0000000000..501ec2b73a --- /dev/null +++ b/tests/auto/corelib/tools/qsharedpointer/nontracked.h @@ -0,0 +1,49 @@ +/**************************************************************************** +** +** Copyright (C) 2013 Intel Corporation. +** Contact: http://www.qt-project.org/legal +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and Digia. For licensing terms and +** conditions see http://qt.digia.com/licensing. For further information +** use the contact form at http://qt.digia.com/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 2.1 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 2.1 requirements +** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Digia gives you certain additional +** rights. These rights are described in the Digia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3.0 as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU General Public License version 3.0 requirements will be +** met: http://www.gnu.org/copyleft/gpl.html. +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#ifndef NONTRACKED_H +#define NONTRACKED_H + +namespace NonTracked { +void dynamicCastFailureNoLeak(); +} + +#endif // NONTRACKED_H diff --git a/tests/auto/corelib/tools/qsharedpointer/qsharedpointer.pro b/tests/auto/corelib/tools/qsharedpointer/qsharedpointer.pro index 3f7f30f474..2e8e335472 100644 --- a/tests/auto/corelib/tools/qsharedpointer/qsharedpointer.pro +++ b/tests/auto/corelib/tools/qsharedpointer/qsharedpointer.pro @@ -5,9 +5,11 @@ QT = core testlib SOURCES = tst_qsharedpointer.cpp \ forwarddeclared.cpp \ + nontracked.cpp \ wrapper.cpp HEADERS = forwarddeclared.h \ + nontracked.h \ wrapper.h TESTDATA += forwarddeclared.cpp forwarddeclared.h diff --git a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp index 9edcb8e787..f9fffaa71c 100644 --- a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp +++ b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp @@ -50,6 +50,7 @@ #include "externaltests.h" #include "forwarddeclared.h" +#include "nontracked.h" #include "wrapper.h" #include @@ -88,6 +89,7 @@ private slots: void dynamicCastDifferentPointers(); void dynamicCastVirtualBase(); void dynamicCastFailure(); + void dynamicCastFailureNoLeak(); #endif void constCorrectness(); void customDeleter(); @@ -1066,6 +1068,11 @@ void tst_QSharedPointer::dynamicCastFailure() QCOMPARE(int(refCountData(baseptr)->weakref.load()), 1); QCOMPARE(int(refCountData(baseptr)->strongref.load()), 1); } + +void tst_QSharedPointer::dynamicCastFailureNoLeak() +{ + NonTracked::dynamicCastFailureNoLeak(); +} #endif void tst_QSharedPointer::constCorrectness()