core: derive incubation controllers from tracked windows list

Replaces the attempts to track incubation controllers directly with a
list of all known windows, then pulls the first usable incubation
controller when an assignment is requested.

This should finally fix incubation controller related use after free crashes.
This commit is contained in:
outfoxxed 2025-09-19 00:16:26 -07:00
parent 996efc93b7
commit b9cce25061
No known key found for this signature in database
GPG key ID: 4C88A185FB89301E
3 changed files with 27 additions and 103 deletions

View file

@ -11,12 +11,12 @@
#include <qlist.h>
#include <qlogging.h>
#include <qloggingcategory.h>
#include <qnamespace.h>
#include <qobject.h>
#include <qqmlcontext.h>
#include <qqmlengine.h>
#include <qqmlerror.h>
#include <qqmlincubator.h>
#include <qquickwindow.h>
#include <qtmetamacros.h>
#include "iconimageprovider.hpp"
@ -242,90 +242,6 @@ void EngineGeneration::onDirectoryChanged() {
}
}
void EngineGeneration::registerIncubationController(QQmlIncubationController* controller) {
// We only want controllers that we can swap out if destroyed.
// This happens if the window owning the active controller dies.
auto* obj = dynamic_cast<QObject*>(controller);
if (!obj) {
qCWarning(logIncubator) << "Could not register incubation controller as it is not a QObject"
<< controller;
return;
}
QObject::connect(
obj,
&QObject::destroyed,
this,
&EngineGeneration::incubationControllerDestroyed,
Qt::UniqueConnection
);
this->incubationControllers.push_back(obj);
qCDebug(logIncubator) << "Registered incubation controller" << obj << "to generation" << this;
// This function can run during destruction.
if (this->engine == nullptr) return;
if (this->engine->incubationController() == &this->delayedIncubationController) {
this->assignIncubationController();
}
}
// Multiple controllers may be destroyed at once. Dynamic casts must be performed before working
// with any controllers. The QQmlIncubationController destructor will already have run by the
// point QObject::destroyed is called, so we can't cast to that.
void EngineGeneration::deregisterIncubationController(QQmlIncubationController* controller) {
auto* obj = dynamic_cast<QObject*>(controller);
if (!obj) {
qCCritical(logIncubator) << "Deregistering incubation controller which is not a QObject, "
"however only QObject controllers should be registered.";
}
QObject::disconnect(obj, nullptr, this, nullptr);
if (this->incubationControllers.removeOne(obj)) {
qCDebug(logIncubator) << "Deregistered incubation controller" << obj << "from" << this;
} else {
qCCritical(logIncubator) << "Failed to deregister incubation controller" << obj << "from"
<< this << "as it was not registered to begin with";
qCCritical(logIncubator) << "Current registered incuabation controllers"
<< this->incubationControllers;
}
// This function can run during destruction.
if (this->engine == nullptr) return;
if (this->engine->incubationController() == controller) {
qCDebug(logIncubator
) << "Destroyed incubation controller was currently active, reassigning from pool";
this->assignIncubationController();
}
}
void EngineGeneration::incubationControllerDestroyed() {
auto* sender = this->sender();
if (this->incubationControllers.removeAll(sender) != 0) {
qCDebug(logIncubator) << "Destroyed incubation controller" << sender << "deregistered from"
<< this;
} else {
qCCritical(logIncubator) << "Destroyed incubation controller" << sender
<< "was not registered, but its destruction was observed by" << this;
return;
}
// This function can run during destruction.
if (this->engine == nullptr) return;
if (dynamic_cast<QObject*>(this->engine->incubationController()) == sender) {
qCDebug(logIncubator
) << "Destroyed incubation controller was currently active, reassigning from pool";
this->assignIncubationController();
}
}
void EngineGeneration::onEngineWarnings(const QList<QQmlError>& warnings) {
for (const auto& error: warnings) {
const auto& url = error.url();
@ -367,13 +283,27 @@ void EngineGeneration::exit(int code) {
this->destroy();
}
void EngineGeneration::assignIncubationController() {
QQmlIncubationController* controller = nullptr;
void EngineGeneration::trackWindowIncubationController(QQuickWindow* window) {
if (this->trackedWindows.contains(window)) return;
if (this->incubationControllersLocked || this->incubationControllers.isEmpty()) {
controller = &this->delayedIncubationController;
} else {
controller = dynamic_cast<QQmlIncubationController*>(this->incubationControllers.first());
QObject::connect(window, &QObject::destroyed, this, &EngineGeneration::onTrackedWindowDestroyed);
this->trackedWindows.append(window);
this->assignIncubationController();
}
void EngineGeneration::onTrackedWindowDestroyed(QObject* object) {
this->trackedWindows.removeAll(static_cast<QQuickWindow*>(object)); // NOLINT
this->assignIncubationController();
}
void EngineGeneration::assignIncubationController() {
QQmlIncubationController* controller = &this->delayedIncubationController;
for (auto* window: this->trackedWindows) {
if (auto* wctl = window->incubationController()) {
controller = wctl;
break;
}
}
qCDebug(logIncubator) << "Assigning incubation controller" << controller << "to generation"

View file

@ -9,6 +9,7 @@
#include <qqmlengine.h>
#include <qqmlerror.h>
#include <qqmlincubator.h>
#include <qquickwindow.h>
#include <qtclasshelpermacros.h>
#include "incubator.hpp"
@ -40,8 +41,7 @@ public:
void setWatchingFiles(bool watching);
bool setExtraWatchedFiles(const QVector<QString>& files);
void registerIncubationController(QQmlIncubationController* controller);
void deregisterIncubationController(QQmlIncubationController* controller);
void trackWindowIncubationController(QQuickWindow* window);
// takes ownership
void registerExtension(const void* key, EngineGenerationExt* extension);
@ -84,13 +84,13 @@ public slots:
private slots:
void onFileChanged(const QString& name);
void onDirectoryChanged();
void incubationControllerDestroyed();
void onTrackedWindowDestroyed(QObject* object);
static void onEngineWarnings(const QList<QQmlError>& warnings);
private:
void postReload();
void assignIncubationController();
QVector<QObject*> incubationControllers;
QVector<QQuickWindow*> trackedWindows;
bool incubationControllersLocked = false;
QHash<const void*, EngineGenerationExt*> extensions;