summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorArno Rehn <a.rehn@menlosystems.com>2023-09-05 09:25:43 +0200
committerQt Cherry-pick Bot <cherrypick_bot@qt-project.org>2023-09-18 12:59:32 +0000
commit34eaba13b87c903715b697ba9df74819c9591b2b (patch)
tree6149ca37893396a0fa70e849d858501d73f44dec
parenta66a3983f213ca3dcfabeef3d68733bcd48748b8 (diff)
QtFuture: Prevent whenAll()/whenAny() from creating reference cycles
whenAll() and whenAny() create a shared context object which is referenced by the continuation lambda. The refcount of context is only correctly managed when it is copied non-const to the lambda's capture list. Fixes: QTBUG-116731 Change-Id: I8e79e1a0dc867f69bbacf1ed873f353a18f6ad38 Reviewed-by: MÃ¥rten Nordheim <marten.nordheim@qt.io> (cherry picked from commit ba2ebc24a1b29020699dc2282b05a60506b56c6d) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> (cherry picked from commit a2792bac1a6077460f288a2b918449011f8c89d6)
-rw-r--r--src/corelib/thread/qfuture_impl.h15
-rw-r--r--tests/auto/corelib/thread/qfuture/tst_qfuture.cpp34
2 files changed, 43 insertions, 6 deletions
diff --git a/src/corelib/thread/qfuture_impl.h b/src/corelib/thread/qfuture_impl.h
index 8f0b282163..bf98071646 100644
--- a/src/corelib/thread/qfuture_impl.h
+++ b/src/corelib/thread/qfuture_impl.h
@@ -1105,9 +1105,10 @@ void addCompletionHandlersImpl(const std::shared_ptr<ContextType> &context,
{
auto future = std::get<Index>(t);
using ResultType = typename ContextType::ValueType;
- future.then([context](const std::tuple_element_t<Index, std::tuple<Ts...>> &f) {
+ // Need context=context so that the compiler does not infer the captured variable's type as 'const'
+ future.then([context=context](const std::tuple_element_t<Index, std::tuple<Ts...>> &f) {
context->checkForCompletion(Index, ResultType { std::in_place_index<Index>, f });
- }).onCanceled([context, future]() {
+ }).onCanceled([context=context, future]() {
context->checkForCompletion(Index, ResultType { std::in_place_index<Index>, future });
});
@@ -1135,9 +1136,10 @@ QFuture<OutputSequence> whenAllImpl(InputIt first, InputIt last)
qsizetype idx = 0;
for (auto it = first; it != last; ++it, ++idx) {
- it->then([context, idx](const ValueType &f) {
+ // Need context=context so that the compiler does not infer the captured variable's type as 'const'
+ it->then([context=context, idx](const ValueType &f) {
context->checkForCompletion(idx, f);
- }).onCanceled([context, idx, f = *it] {
+ }).onCanceled([context=context, idx, f = *it] {
context->checkForCompletion(idx, f);
});
}
@@ -1175,9 +1177,10 @@ QFuture<QtFuture::WhenAnyResult<typename Future<ValueType>::type>> whenAnyImpl(I
qsizetype idx = 0;
for (auto it = first; it != last; ++it, ++idx) {
- it->then([context, idx](const ValueType &f) {
+ // Need context=context so that the compiler does not infer the captured variable's type as 'const'
+ it->then([context=context, idx](const ValueType &f) {
context->checkForCompletion(idx, QtFuture::WhenAnyResult { idx, f });
- }).onCanceled([context, idx, f = *it] {
+ }).onCanceled([context=context, idx, f = *it] {
context->checkForCompletion(idx, QtFuture::WhenAnyResult { idx, f });
});
}
diff --git a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
index 0bdeb96b14..fa01886182 100644
--- a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
+++ b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
@@ -5012,6 +5012,40 @@ void tst_QFuture::continuationsDontLeak()
QVERIFY(continuationIsRun);
}
QCOMPARE(InstanceCounter::count, 0);
+
+ {
+ // QTBUG-116731: Must pass with ASan enabled
+ bool continuationIsRun = false;
+ auto f = QtFuture::makeReadyValueFuture(42);
+ QtFuture::whenAll(f).then([&](auto) { continuationIsRun = true; });
+ QVERIFY(continuationIsRun);
+ }
+
+ {
+ // QTBUG-116731: Must pass with ASan enabled
+ bool continuationIsRun = false;
+ auto f = QtFuture::makeReadyValueFuture(42);
+ QList fs{f};
+ QtFuture::whenAll(fs.begin(), fs.end()).then([&](auto) { continuationIsRun = true; });
+ QVERIFY(continuationIsRun);
+ }
+
+ {
+ // QTBUG-116731: Must pass with ASan enabled
+ bool continuationIsRun = false;
+ auto f = QtFuture::makeReadyValueFuture(42);
+ QtFuture::whenAny(f).then([&](auto) { continuationIsRun = true; });
+ QVERIFY(continuationIsRun);
+ }
+
+ {
+ // QTBUG-116731: Must pass with ASan enabled
+ bool continuationIsRun = false;
+ auto f = QtFuture::makeReadyValueFuture(42);
+ QList fs{f};
+ QtFuture::whenAny(fs.begin(), fs.end()).then([&](auto) { continuationIsRun = true; });
+ QVERIFY(continuationIsRun);
+ }
}
// This test checks that we do not get use-after-free