[neon/qt6/qt6-declarative/Neon/release] debian/patches: add patches recommended by fvogt
Jonathan Riddell
null at kde.org
Fri Nov 15 09:53:57 GMT 2024
Git commit 39328154cf82c57657fbcb3cc56b116d497725a1 by Jonathan Riddell.
Committed on 15/11/2024 at 09:53.
Pushed by jriddell into branch 'Neon/release'.
add patches recommended by fvogt
A +338 -0 debian/patches/qtdeclarative.git-104b0d6e88ce6781c9d31cf0dd14dfe99988b789.patch
A +124 -0 debian/patches/qtdeclarative.git-fbdbed919f59cc7b4520f5aab2149f4c99b63e24.patch
M +7 -1 debian/patches/series
https://invent.kde.org/neon/qt6/qt6-declarative/-/commit/39328154cf82c57657fbcb3cc56b116d497725a1
diff --git a/debian/patches/qtdeclarative.git-104b0d6e88ce6781c9d31cf0dd14dfe99988b789.patch b/debian/patches/qtdeclarative.git-104b0d6e88ce6781c9d31cf0dd14dfe99988b789.patch
new file mode 100644
index 0000000..c0f0ff2
--- /dev/null
+++ b/debian/patches/qtdeclarative.git-104b0d6e88ce6781c9d31cf0dd14dfe99988b789.patch
@@ -0,0 +1,338 @@
+From 104b0d6e88ce6781c9d31cf0dd14dfe99988b789 Mon Sep 17 00:00:00 2001
+From: Luca Di Sera <luca.disera at qt.io>
+Date: Fri, 30 Aug 2024 17:44:16 +0200
+Subject: [PATCH] Log state transitions for the GC
+
+Add a new logging category, "qt.qml.gc.stateTransitions", that can be
+enabled to receive some simple log lines about the GC being on the
+verge of executing the current state and about the new state that it
+transitioned to after the execution.
+
+As the new logs print the current state, which is stored through the
+`GCState` enum, slightly modify the code to allow for registering the
+enumeration with `Q_ENUM`.
+
+In particular, move `GCState` and `GCStateInfo`, the latter due to the
+dependencies between the types, under `GCStateMachine`.
+Make `GCStateMachine` a `Q_GADGET` and `GCState` a `Q_ENUM` and provide
+some using statements to reduce the impact of the changes.
+
+Finally, fix the unqualified accesses to the variants of `GCState` to
+support the new structure.
+
+Task-number: QTBUG-128357
+Change-Id: I9d469ddb745f70b9c4553379f6d96719b3a2bb09
+Reviewed-by: Fabian Kosmale <fabian.kosmale at qt.io>
+(cherry picked from commit c048f8337c7b0aa8a31c93e559aa8a5bf286c29e)
+---
+ src/qml/memory/qv4mm.cpp | 60 +++++++++++++++++++++++++++------------------
+ src/qml/memory/qv4mm_p.h | 64 ++++++++++++++++++++++++++++--------------------
+ 2 files changed, 73 insertions(+), 51 deletions(-)
+
+diff --git a/src/qml/memory/qv4mm.cpp b/src/qml/memory/qv4mm.cpp
+index 0ef32d5273a..d9f4541aec9 100644
+--- a/src/qml/memory/qv4mm.cpp
++++ b/src/qml/memory/qv4mm.cpp
+@@ -60,6 +60,8 @@ Q_LOGGING_CATEGORY(lcGcStats, "qt.qml.gc.statistics")
+ Q_DECLARE_LOGGING_CATEGORY(lcGcStats)
+ Q_LOGGING_CATEGORY(lcGcAllocatorStats, "qt.qml.gc.allocatorStats")
+ Q_DECLARE_LOGGING_CATEGORY(lcGcAllocatorStats)
++Q_LOGGING_CATEGORY(lcGcStateTransitions, "qt.qml.gc.stateTransitions")
++Q_DECLARE_LOGGING_CATEGORY(lcGcStateTransitions)
+
+ using namespace WTF;
+
+@@ -680,27 +682,27 @@ GCState markStart(GCStateMachine *that, ExtraData &)
+ //Initialize the mark stack
+ that->mm->m_markStack = std::make_unique<MarkStack>(that->mm->engine);
+ that->mm->engine->isGCOngoing = true;
+- return MarkGlobalObject;
++ return GCState::MarkGlobalObject;
+ }
+
+ GCState markGlobalObject(GCStateMachine *that, ExtraData &)
+ {
+ that->mm->engine->markObjects(that->mm->m_markStack.get());
+- return MarkJSStack;
++ return GCState::MarkJSStack;
+ }
+
+ GCState markJSStack(GCStateMachine *that, ExtraData &)
+ {
+ that->mm->collectFromJSStack(that->mm->markStack());
+- return InitMarkPersistentValues;
++ return GCState::InitMarkPersistentValues;
+ }
+
+ GCState initMarkPersistentValues(GCStateMachine *that, ExtraData &stateData)
+ {
+ if (!that->mm->m_persistentValues)
+- return InitMarkWeakValues; // no persistent values to mark
++ return GCState::InitMarkWeakValues; // no persistent values to mark
+ stateData = GCIteratorStorage { that->mm->m_persistentValues->begin() };
+- return MarkPersistentValues;
++ return GCState::MarkPersistentValues;
+ }
+
+ static constexpr int markLoopIterationCount = 1024;
+@@ -717,35 +719,35 @@ bool wasDrainNecessary(MarkStack *ms, QDeadlineTimer deadline)
+ GCState markPersistentValues(GCStateMachine *that, ExtraData &stateData) {
+ auto markStack = that->mm->markStack();
+ if (wasDrainNecessary(markStack, that->deadline) && that->deadline.hasExpired())
+- return MarkPersistentValues;
++ return GCState::MarkPersistentValues;
+ PersistentValueStorage::Iterator& it = get<GCIteratorStorage>(stateData).it;
+ // avoid repeatedly hitting the timer constantly by batching iterations
+ for (int i = 0; i < markLoopIterationCount; ++i) {
+ if (!it.p)
+- return InitMarkWeakValues;
++ return GCState::InitMarkWeakValues;
+ if (Managed *m = (*it).as<Managed>())
+ m->mark(markStack);
+ ++it;
+ }
+- return MarkPersistentValues;
++ return GCState::MarkPersistentValues;
+ }
+
+ GCState initMarkWeakValues(GCStateMachine *that, ExtraData &stateData)
+ {
+ stateData = GCIteratorStorage { that->mm->m_weakValues->begin() };
+- return MarkWeakValues;
++ return GCState::MarkWeakValues;
+ }
+
+ GCState markWeakValues(GCStateMachine *that, ExtraData &stateData)
+ {
+ auto markStack = that->mm->markStack();
+ if (wasDrainNecessary(markStack, that->deadline) && that->deadline.hasExpired())
+- return MarkWeakValues;
++ return GCState::MarkWeakValues;
+ PersistentValueStorage::Iterator& it = get<GCIteratorStorage>(stateData).it;
+ // avoid repeatedly hitting the timer constantly by batching iterations
+ for (int i = 0; i < markLoopIterationCount; ++i) {
+ if (!it.p)
+- return MarkDrain;
++ return GCState::MarkDrain;
+ QObjectWrapper *qobjectWrapper = (*it).as<QObjectWrapper>();
+ ++it;
+ if (!qobjectWrapper)
+@@ -766,25 +768,25 @@ GCState markWeakValues(GCStateMachine *that, ExtraData &stateData)
+ if (keepAlive)
+ qobjectWrapper->mark(that->mm->markStack());
+ }
+- return MarkWeakValues;
++ return GCState::MarkWeakValues;
+ }
+
+ GCState markDrain(GCStateMachine *that, ExtraData &)
+ {
+ if (that->deadline.isForever()) {
+ that->mm->markStack()->drain();
+- return MarkReady;
++ return GCState::MarkReady;
+ }
+ auto drainState = that->mm->m_markStack->drain(that->deadline);
+ return drainState == MarkStack::DrainState::Complete
+- ? MarkReady
+- : MarkDrain;
++ ? GCState::MarkReady
++ : GCState::MarkDrain;
+ }
+
+ GCState markReady(GCStateMachine *, ExtraData &)
+ {
+ //Possibility to do some clean up, stat printing, etc...
+- return InitCallDestroyObjects;
++ return GCState::InitCallDestroyObjects;
+ }
+
+ /** \!internal
+@@ -801,9 +803,9 @@ GCState initCallDestroyObjects(GCStateMachine *that, ExtraData &stateData)
+ // as we don't have a deletion barrier, we need to rescan the stack
+ redrain(that);
+ if (!that->mm->m_weakValues)
+- return FreeWeakMaps; // no need to call destroy objects
++ return GCState::FreeWeakMaps; // no need to call destroy objects
+ stateData = GCIteratorStorage { that->mm->m_weakValues->begin() };
+- return CallDestroyObjects;
++ return GCState::CallDestroyObjects;
+ }
+ GCState callDestroyObject(GCStateMachine *that, ExtraData &stateData)
+ {
+@@ -816,7 +818,7 @@ GCState callDestroyObject(GCStateMachine *that, ExtraData &stateData)
+ // avoid repeatedly hitting the timer constantly by batching iterations
+ for (int i = 0; i < markLoopIterationCount; ++i) {
+ if (!it.p)
+- return FreeWeakMaps;
++ return GCState::FreeWeakMaps;
+ Managed *m = (*it).managed();
+ ++it;
+ if (!m || m->markBit())
+@@ -826,7 +828,7 @@ GCState callDestroyObject(GCStateMachine *that, ExtraData &stateData)
+ if (QObjectWrapper *qobjectWrapper = m->as<QObjectWrapper>())
+ qobjectWrapper->destroyObject(/*lastSweep =*/false);
+ }
+- return CallDestroyObjects;
++ return GCState::CallDestroyObjects;
+ }
+
+ void freeWeakMaps(MemoryManager *mm)
+@@ -843,7 +845,7 @@ void freeWeakMaps(MemoryManager *mm)
+ GCState freeWeakMaps(GCStateMachine *that, ExtraData &)
+ {
+ freeWeakMaps(that->mm);
+- return FreeWeakSets;
++ return GCState::FreeWeakSets;
+ }
+
+ void freeWeakSets(MemoryManager *mm)
+@@ -861,13 +863,13 @@ void freeWeakSets(MemoryManager *mm)
+ GCState freeWeakSets(GCStateMachine *that, ExtraData &)
+ {
+ freeWeakSets(that->mm);
+- return HandleQObjectWrappers;
++ return GCState::HandleQObjectWrappers;
+ }
+
+ GCState handleQObjectWrappers(GCStateMachine *that, ExtraData &)
+ {
+ that->mm->cleanupDeletedQObjectWrappersInSweep();
+- return DoSweep;
++ return GCState::DoSweep;
+ }
+
+ GCState doSweep(GCStateMachine *that, ExtraData &)
+@@ -891,7 +893,7 @@ GCState doSweep(GCStateMachine *that, ExtraData &)
+
+ mm->updateUnmanagedHeapSizeGCLimit();
+
+- return Invalid;
++ return GCState::Invalid;
+ }
+
+ }
+@@ -1491,8 +1493,12 @@ void GCStateMachine::transition() {
+ */
+ redrain(this);
+ }
++ qCDebug(lcGcStateTransitions) << "Preparing to execute the"
++ << QMetaEnum::fromType<GCState>().key(state) << "state";
+ GCStateInfo& stateInfo = stateInfoMap[int(state)];
+ state = stateInfo.execute(this, stateData);
++ qCDebug(lcGcStateTransitions) << "Transitioning to the"
++ << QMetaEnum::fromType<GCState>().key(state) << "state";
+ if (stateInfo.breakAfter)
+ break;
+ }
+@@ -1505,8 +1511,12 @@ void GCStateMachine::transition() {
+ } else {
+ deadline = QDeadlineTimer::Forever;
+ while (state != GCState::Invalid) {
++ qCDebug(lcGcStateTransitions) << "Preparing to execute the"
++ << QMetaEnum::fromType<GCState>().key(state) << "state";
+ GCStateInfo& stateInfo = stateInfoMap[int(state)];
+ state = stateInfo.execute(this, stateData);
++ qCDebug(lcGcStateTransitions) << "Transitioning to the"
++ << QMetaEnum::fromType<GCState>().key(state) << "state";
+ }
+ }
+ }
+@@ -1514,3 +1524,5 @@ void GCStateMachine::transition() {
+ } // namespace QV4
+
+ QT_END_NAMESPACE
++
++#include "moc_qv4mm_p.cpp"
+diff --git a/src/qml/memory/qv4mm_p.h b/src/qml/memory/qv4mm_p.h
+index ef0cd0c36c5..9cdebdb1f47 100644
+--- a/src/qml/memory/qv4mm_p.h
++++ b/src/qml/memory/qv4mm_p.h
+@@ -28,40 +28,48 @@ QT_BEGIN_NAMESPACE
+
+ namespace QV4 {
+
+-enum GCState {
+- MarkStart = 0,
+- MarkGlobalObject,
+- MarkJSStack,
+- InitMarkPersistentValues,
+- MarkPersistentValues,
+- InitMarkWeakValues,
+- MarkWeakValues,
+- MarkDrain,
+- MarkReady,
+- InitCallDestroyObjects,
+- CallDestroyObjects,
+- FreeWeakMaps,
+- FreeWeakSets,
+- HandleQObjectWrappers,
+- DoSweep,
+- Invalid,
+- Count,
+-};
+-
+ struct GCData { virtual ~GCData(){};};
+
+ struct GCIteratorStorage {
+ PersistentValueStorage::Iterator it{nullptr, 0};
+ };
+-struct GCStateMachine;
+-
+-struct GCStateInfo {
+- using ExtraData = std::variant<std::monostate, GCIteratorStorage>;
+- GCState (*execute)(GCStateMachine *, ExtraData &) = nullptr; // Function to execute for this state, returns true if ready to transition
+- bool breakAfter{false};
+-};
+
+ struct GCStateMachine {
++ Q_GADGET_EXPORT(Q_QML_EXPORT)
++
++public:
++ enum GCState {
++ MarkStart = 0,
++ MarkGlobalObject,
++ MarkJSStack,
++ InitMarkPersistentValues,
++ MarkPersistentValues,
++ InitMarkWeakValues,
++ MarkWeakValues,
++ MarkDrain,
++ MarkReady,
++ InitCallDestroyObjects,
++ CallDestroyObjects,
++ FreeWeakMaps,
++ FreeWeakSets,
++ HandleQObjectWrappers,
++ DoSweep,
++ Invalid,
++ Count,
++ };
++ Q_ENUM(GCState)
++
++ struct StepTiming {
++ qint64 rolling_sum = 0;
++ qint64 count = 0;
++ };
++
++ struct GCStateInfo {
++ using ExtraData = std::variant<std::monostate, GCIteratorStorage>;
++ GCState (*execute)(GCStateMachine *, ExtraData &) = nullptr; // Function to execute for this state, returns true if ready to transition
++ bool breakAfter{false};
++ };
++
+ using ExtraData = GCStateInfo::ExtraData;
+ GCState state{GCState::Invalid};
+ std::chrono::microseconds timeLimit{};
+@@ -94,6 +102,8 @@ struct GCStateMachine {
+ }
+ };
+
++using GCState = GCStateMachine::GCState;
++using GCStateInfo = GCStateMachine::GCStateInfo;
+
+ struct ChunkAllocator;
+ struct MemorySegment;
+--
+2.16.3
+
diff --git a/debian/patches/qtdeclarative.git-fbdbed919f59cc7b4520f5aab2149f4c99b63e24.patch b/debian/patches/qtdeclarative.git-fbdbed919f59cc7b4520f5aab2149f4c99b63e24.patch
new file mode 100644
index 0000000..bf71190
--- /dev/null
+++ b/debian/patches/qtdeclarative.git-fbdbed919f59cc7b4520f5aab2149f4c99b63e24.patch
@@ -0,0 +1,124 @@
+From fbdbed919f59cc7b4520f5aab2149f4c99b63e24 Mon Sep 17 00:00:00 2001
+From: =?utf8?q?Olivier=20De=20Canni=C3=A8re?= <olivier.decanniere at qt.io>
+Date: Mon, 11 Nov 2024 13:23:39 +0100
+Subject: [PATCH] Engine: Mark created wrapped objects after
+ GCState::MarkWeakValues
+
+If an object is wrapped after the MarkWeakValues state of the GC but
+before the sweep, it will not have been marked and will be collected.
+
+We need to mark wrapped objects when they are created in those cases.
+
+Fixes: QTBUG-130767
+Change-Id: I2fd7c8829267a2e3de1ac374859a4d21d948dd8f
+Reviewed-by: Ulf Hermann <ulf.hermann at qt.io>
+(cherry picked from commit d148d8d78419effdc28b6bf74704379aa9a5950a)
+Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot at qt-project.org>
+---
+ src/qml/jsruntime/qv4qobjectwrapper_p.h | 21 +++++++++++++++++--
+ tests/auto/qml/qv4mm/tst_qv4mm.cpp | 36 +++++++++++++++++++++++++++++++++
+ 2 files changed, 55 insertions(+), 2 deletions(-)
+
+diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h
+index 826930cd255..a2538e7d5d5 100644
+--- a/src/qml/jsruntime/qv4qobjectwrapper_p.h
++++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h
+@@ -222,6 +222,19 @@ private:
+
+ Q_DECLARE_OPERATORS_FOR_FLAGS(QObjectWrapper::Flags)
+
++// We generally musn't pass ReturnedValue as arguments to other functions.
++// In this case, we do it solely for marking purposes so it's fine.
++inline void markIfPastMarkWeakValues(ExecutionEngine *engine, ReturnedValue rv)
++{
++ const auto gcState = engine->memoryManager->gcStateMachine->state;
++ if (gcState != GCStateMachine::Invalid && gcState >= GCState::MarkWeakValues) {
++ QV4::WriteBarrier::markCustom(engine, [rv](QV4::MarkStack *ms) {
++ auto *m = StaticValue::fromReturnedValue(rv).m();
++ m->mark(ms);
++ });
++ }
++}
++
+ inline ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *object)
+ {
+ if (Q_UNLIKELY(QQmlData::wasDeleted(object)))
+@@ -233,7 +246,9 @@ inline ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *obje
+ return ddata->jsWrapper.value();
+ }
+
+- return wrap_slowPath(engine, object);
++ const auto rv = wrap_slowPath(engine, object);
++ markIfPastMarkWeakValues(engine, rv);
++ return rv;
+ }
+
+ // Unfortunately we still need a non-const QObject* here because QQmlData needs to register itself in QObjectPrivate.
+@@ -242,7 +257,9 @@ inline ReturnedValue QObjectWrapper::wrapConst(ExecutionEngine *engine, QObject
+ if (Q_UNLIKELY(QQmlData::wasDeleted(object)))
+ return QV4::Encode::null();
+
+- return wrapConst_slowPath(engine, object);
++ const auto rv = wrapConst_slowPath(engine, object);
++ markIfPastMarkWeakValues(engine, rv);
++ return rv;
+ }
+
+ inline bool canConvert(const QQmlPropertyCache *fromMo, const QQmlPropertyCache *toMo)
+diff --git a/tests/auto/qml/qv4mm/tst_qv4mm.cpp b/tests/auto/qml/qv4mm/tst_qv4mm.cpp
+index 7859d105510..6f502c73310 100644
+--- a/tests/auto/qml/qv4mm/tst_qv4mm.cpp
++++ b/tests/auto/qml/qv4mm/tst_qv4mm.cpp
+@@ -46,6 +46,7 @@ private slots:
+ void jittedStoreLocalMarksValue();
+ void forInOnProxyMarksTarget();
+ void allocWithMemberDataMidwayDrain();
++ void markObjectWrappersAfterMarkWeakValues();
+ };
+
+ tst_qv4mm::tst_qv4mm()
+@@ -795,6 +796,41 @@ void tst_qv4mm::allocWithMemberDataMidwayDrain()
+ QVERIFY(o); // dummy check
+ }
+
++void tst_qv4mm::markObjectWrappersAfterMarkWeakValues()
++{
++ // Advance gc to just after MarkWeakValues
++ const auto setupGC = [](QV4::ExecutionEngine *v4) {
++ QCOMPARE(v4->memoryManager->gcBlocked, QV4::MemoryManager::Unblocked);
++ auto sm = v4->memoryManager->gcStateMachine.get();
++ sm->reset();
++ v4->memoryManager->gcBlocked = QV4::MemoryManager::NormalBlocked;
++ const QV4::GCState targetState = QV4::GCState(QV4::GCState::MarkWeakValues + 1);
++ while (sm->state != targetState) {
++ QV4::GCStateInfo& stateInfo = sm->stateInfoMap[int(sm->state)];
++ sm->state = stateInfo.execute(sm, sm->stateData);
++ }
++ QCOMPARE(sm->state, targetState);
++ };
++
++ QQmlEngine engine;
++ QV4::ExecutionEngine *v4 = engine.handle();
++ setupGC(v4);
++
++ QObject *object = new QObject;
++ object->setObjectName("yep");
++ QJSEngine::setObjectOwnership(object, QJSEngine::JavaScriptOwnership);
++ engine.rootContext()->setContextProperty("prop", object);
++ (void) QV4::QObjectWrapper::wrap(v4, object);
++ QVERIFY(v4->memoryManager->tryForceGCCompletion());
++
++ QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
++ QCoreApplication::processEvents();
++
++ const QVariant retrieved = engine.rootContext()->contextProperty("prop");
++ QVERIFY(qvariant_cast<QObject *>(retrieved));
++ QCOMPARE(qvariant_cast<QObject *>(retrieved)->objectName(), "yep");
++}
++
+ QTEST_MAIN(tst_qv4mm)
+
+ #include "tst_qv4mm.moc"
+--
+2.16.3
+
diff --git a/debian/patches/series b/debian/patches/series
index 14194d6..dc74a0f 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,4 +5,10 @@ network_applet_list_2.diff
#carl reported https://codereview.qt-project.org/c/qt/qtdeclarative/+/579714 for backports
# but superceded by https://codereview.qt-project.org/c/qt/qtdeclarative/+/595278
-kirigami_crashes.diff
\ No newline at end of file
+kirigami_crashes.diff
+
+# https://codereview.qt-project.org/c/qt/qtdeclarative/+/604180
+# Engine: Mark created wrapped objects after GCState::MarkWeakValues
+# to fix https://bugs.kde.org/show_bug.cgi?id=494804 and likely some others
+qtdeclarative.git-104b0d6e88ce6781c9d31cf0dd14dfe99988b789.patch
+qtdeclarative.git-fbdbed919f59cc7b4520f5aab2149f4c99b63e24.patch
More information about the Neon-commits
mailing list