From ba511b2fa4782d6618a5261bbbd50f0c57266a3a Mon Sep 17 00:00:00 2001 From: Andrei Golubev Date: Fri, 9 Oct 2020 11:01:16 +0200 Subject: Reject overwrites by the same index in QPromise::addResult() One can call addResult(value, index) twice and consequently set the value twice by the same index. This seems rather strange and probably should not be allowed. This commit rejects setting results when there's already a valid result by that index. Consequently, this fixes memory leaks caused by N-times-called addResult(..., index) Fixes: QTBUG-86828 Change-Id: I77494f2cb73ce727ffad721cfcdcaa420899eb25 Reviewed-by: Sona Kurazyan --- src/corelib/thread/qfutureinterface.h | 17 ++++++++++------- src/corelib/thread/qpromise.qdoc | 3 +++ src/corelib/thread/qresultstore.cpp | 14 ++++++++++++++ src/corelib/thread/qresultstore.h | 23 ++++++++++++++++++++++- 4 files changed, 49 insertions(+), 8 deletions(-) (limited to 'src/corelib/thread') diff --git a/src/corelib/thread/qfutureinterface.h b/src/corelib/thread/qfutureinterface.h index 6c7c511cc0..9296c63f0b 100644 --- a/src/corelib/thread/qfutureinterface.h +++ b/src/corelib/thread/qfutureinterface.h @@ -270,11 +270,12 @@ inline void QFutureInterface::reportResult(const T *result, int index) if (store.filterMode()) { const int resultCountBefore = store.count(); - store.addResult(index, result); - this->reportResultsReady(resultCountBefore, store.count()); + if (store.addResult(index, result) != -1) + this->reportResultsReady(resultCountBefore, store.count()); } else { const int insertIndex = store.addResult(index, result); - this->reportResultsReady(insertIndex, insertIndex + 1); + if (insertIndex != -1) + this->reportResultsReady(insertIndex, insertIndex + 1); } } @@ -289,7 +290,8 @@ void QFutureInterface::reportAndMoveResult(T &&result, int index) const int oldResultCount = store.count(); const int insertIndex = store.moveResult(index, std::forward(result)); - if (!store.filterMode() || oldResultCount < store.count()) // Let's make sure it's not in pending results. + // Let's make sure it's not in pending results. + if (insertIndex != -1 && (!store.filterMode() || oldResultCount < store.count())) reportResultsReady(insertIndex, store.count()); } @@ -317,11 +319,12 @@ inline void QFutureInterface::reportResults(const QList &_results, int beg if (store.filterMode()) { const int resultCountBefore = store.count(); - store.addResults(beginIndex, &_results, count); - this->reportResultsReady(resultCountBefore, store.count()); + if (store.addResults(beginIndex, &_results, count) != -1) + this->reportResultsReady(resultCountBefore, store.count()); } else { const int insertIndex = store.addResults(beginIndex, &_results, count); - this->reportResultsReady(insertIndex, insertIndex + _results.count()); + if (insertIndex != -1) + this->reportResultsReady(insertIndex, insertIndex + _results.count()); } } diff --git a/src/corelib/thread/qpromise.qdoc b/src/corelib/thread/qpromise.qdoc index 9c7925da95..4b7b497b3f 100644 --- a/src/corelib/thread/qpromise.qdoc +++ b/src/corelib/thread/qpromise.qdoc @@ -112,6 +112,9 @@ Adds \a result to the internal result collection at \a index position. If index is unspecified, \a result is added to the end of the collection. + \note addResult() rejects \a result if there's already another result in the + collection stored at the same index. + You can get a result at a specific index by calling QFuture::resultAt(). \note It is possible to specify an arbitrary index and request result at diff --git a/src/corelib/thread/qresultstore.cpp b/src/corelib/thread/qresultstore.cpp index b6af27dfa9..a239954dbe 100644 --- a/src/corelib/thread/qresultstore.cpp +++ b/src/corelib/thread/qresultstore.cpp @@ -144,6 +144,11 @@ bool ResultIteratorBase::canIncrementVectorIndex() const return (m_vectorIndex + 1 < mapIterator.value().m_count); } +bool ResultIteratorBase::isValid() const +{ + return mapIterator.value().isValid(); +} + ResultStoreBase::ResultStoreBase() : insertIndex(0), resultCount(0), m_filterMode(false), filteredResults(0) { } @@ -196,6 +201,15 @@ int ResultStoreBase::insertResultItem(int index, ResultItem &resultItem) return storeIndex; } +bool ResultStoreBase::containsValidResultItem(int index) const +{ + // index might refer to either visible or pending result + const bool inPending = m_filterMode && index != -1 && index > insertIndex; + const auto &store = inPending ? pendingResults : m_results; + auto it = findResult(store, index); + return it != ResultIteratorBase(store.end()) && it.isValid(); +} + void ResultStoreBase::syncPendingResults() { // check if we can insert any of the pending results: diff --git a/src/corelib/thread/qresultstore.h b/src/corelib/thread/qresultstore.h index 5f8c7f150a..0a1382fd79 100644 --- a/src/corelib/thread/qresultstore.h +++ b/src/corelib/thread/qresultstore.h @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. +** Copyright (C) 2020 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -87,6 +87,8 @@ public: bool operator!=(const ResultIteratorBase &other) const; bool isVector() const; bool canIncrementVectorIndex() const; + bool isValid() const; + protected: QMap::const_iterator mapIterator; int m_vectorIndex; @@ -139,6 +141,7 @@ public: protected: int insertResultItem(int index, ResultItem &resultItem); void insertResultItemIfValid(int index, ResultItem &resultItem); + bool containsValidResultItem(int index) const; void syncPendingResults(); void syncResultCount(); int updateInsertIndex(int index, int _count); @@ -169,6 +172,9 @@ public: template int addResult(int index, const T *result) { + if (containsValidResultItem(index)) // reject if already present + return -1; + if (result == nullptr) return addResult(index, static_cast(nullptr)); @@ -178,18 +184,27 @@ public: template int moveResult(int index, T &&result) { + if (containsValidResultItem(index)) // reject if already present + return -1; + return addResult(index, static_cast(new T(std::move_if_noexcept(result)))); } template int addResults(int index, const QList *results) { + if (containsValidResultItem(index)) // reject if already present + return -1; + return addResults(index, new QList(*results), results->count(), results->count()); } template int addResults(int index, const QList *results, int totalCount) { + if (containsValidResultItem(index)) // reject if already present + return -1; + if (m_filterMode == true && results->count() != totalCount && 0 == results->count()) return addResults(index, nullptr, 0, totalCount); @@ -198,12 +213,18 @@ public: int addCanceledResult(int index) { + if (containsValidResultItem(index)) // reject if already present + return -1; + return addResult(index, static_cast(nullptr)); } template int addCanceledResults(int index, int _count) { + if (containsValidResultItem(index)) // reject if already present + return -1; + QList empty; return addResults(index, &empty, _count); } -- cgit v1.2.3