From e46c3c3e5ab4f9b54089b75d19249f4882e2434c Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 14 Jul 2023 14:22:29 +0200 Subject: [PATCH] QCborValue: fix memleak on Array to Map coercion When converting from an array to a map, we double the number of elements, spread the old elements out to make one free slot of space in front of each, and then place Integer values counting from 0 into the free slots. The old code contained a loop that would add a strong reference to the original elements that happen to be containers and thus are ref-counted in the first place. But this additional strong reference is not needed: In both cases, detached or in-place, the detach() call that ensured unique ownership of 'map/dst' will have either directly or indirectly updated the ref-counts of the elements correctly, and the following loops just reshuffle the elements in the QList, they don't create new copies (QtCbor::Elements doesn't by itself manipulate container->ref, but even if it did, the copy would have increased, and the assignment of the Integers would have decreased, the ref-count again). Adding the strong ref without a user then caused the container members to be leaked. Fix by removing the loop. [ChangeLog][QtCore][QCborValue] Fixed a memory leak when an array was coerced into a map. Amends ccea34464075759424e61806c7bc98ee3e658670. Not picking to 6.4 as it's closed at this time. Fixes: QTBUG-115249 Change-Id: I369c372e91c3f0cfe3c65f9b0ea8507d08fdaf48 Reviewed-by: Thiago Macieira (cherry picked from commit ecab68989e623737f7f930d7b123471ccffbfb95) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/serialization/qcborvalue.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index 2a01d7cfb5d..a1c56dfca4b 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -2248,12 +2248,6 @@ static void convertArrayToMap(QCborContainerPrivate *&array) for (qsizetype i = 0; i < size; ++i) dst[i * 2] = { i, QCborValue::Integer }; - // only do this last portion if we're not modifying in-place - for (qsizetype i = 0; src != dst && i < size; ++i) { - if (dst[i * 2 + 1].flags & QtCbor::Element::IsContainer) - dst[i * 2 + 1].container->ref.ref(); - } - // update reference counts assignContainer(array, map); }