From fc66eca9bd61bdf10de0cfea2ec7c898fdec8c1d Mon Sep 17 00:00:00 2001 From: Nils Schimmelmann Date: Sun, 19 Apr 2026 21:18:07 -0500 Subject: [PATCH] fix Mac teardown crashes by correcting QObject ownership hierarchy The application was crashing on macOS during shutdown due to improper parenting of UI components. When objects like delegates or models are parented to the main Widget instead of the View, they can be destroyed out of order, leading to use-after-free errors when the View attempts to access them during its own destruction. - Parent GroupModel and GroupProxyModel to m_table instead of 'this'. - Parent GroupDelegate and TimerDelegate to their respective views. - Ensure the lifecycle of the delegate matches the lifecycle of the view to prevent segmentation faults during signal disconnection. - Restore heap allocation for GroupModel to ensure Qt's ownership system manages its cleanup correctly. --- src/group/groupwidget.cpp | 36 +++++++++++++++++------------------- src/group/groupwidget.h | 6 +++--- src/timers/TimerWidget.cpp | 4 ++-- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/group/groupwidget.cpp b/src/group/groupwidget.cpp index 0878ee206..3cdb32d67 100644 --- a/src/group/groupwidget.cpp +++ b/src/group/groupwidget.cpp @@ -829,14 +829,7 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget : QWidget(parent) , m_group(group) , m_map(md) - , m_model(this) { - if (m_group) { - m_model.setCharacters(m_group->selectAll()); - } else { - m_model.setCharacters({}); - } - auto *layout = new QVBoxLayout(this); layout->setAlignment(Qt::AlignTop); layout->setContentsMargins(0, 0, 0, 0); @@ -849,8 +842,15 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget m_table->horizontalHeader()->setStretchLastSection(true); m_table->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents); - m_proxyModel = new GroupProxyModel(this); - m_proxyModel->setSourceModel(&m_model); + m_model = new GroupModel(m_table); + if (m_group) { + m_model->setCharacters(m_group->selectAll()); + } else { + m_model->setCharacters({}); + } + + m_proxyModel = new GroupProxyModel(m_table); + m_proxyModel->setSourceModel(m_model); m_table->setModel(m_proxyModel); m_table->setDragEnabled(true); @@ -859,7 +859,7 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget m_table->setDefaultDropAction(Qt::MoveAction); m_table->setDropIndicatorShown(true); - m_table->setItemDelegate(new GroupDelegate(this)); + m_table->setItemDelegate(new GroupDelegate(m_table)); layout->addWidget(m_table); m_pulseTimer = new QTimer(this); @@ -912,7 +912,7 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget return; } - selectedCharacter = m_model.getCharacter(sourceIndex.row()); + selectedCharacter = deref(m_model).getCharacter(sourceIndex.row()); if (selectedCharacter) { // Build Context menu m_center->setText( @@ -947,8 +947,6 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget GroupWidget::~GroupWidget() { m_pulseTimer->stop(); - delete m_table; - delete m_recolor; } QSize GroupWidget::sizeHint() const @@ -964,7 +962,7 @@ void GroupWidget::updateColumnVisibility() { // Hide unnecessary columns like mana if everyone is a zorc/troll const auto one_character_had_mana = [this]() -> bool { - for (const auto &character : m_model.getCharacters()) { + for (const auto &character : deref(m_model).getCharacters()) { if (character && (character->getMana() > 0 || character->getMaxMana() > 0)) { return true; } @@ -978,7 +976,7 @@ void GroupWidget::updateColumnVisibility() void GroupWidget::updatePulseTimer() { const auto needs_pulse = [this]() -> bool { - for (const auto &character : m_model.getCharacters()) { + for (const auto &character : deref(m_model).getCharacters()) { if (!character) { continue; } @@ -1012,7 +1010,7 @@ void GroupWidget::updatePulseTimer() void GroupWidget::slot_onCharacterAdded(SharedGroupChar character) { assert(character); - m_model.insertCharacter(character); + deref(m_model).insertCharacter(character); updateColumnVisibility(); updatePulseTimer(); } @@ -1020,7 +1018,7 @@ void GroupWidget::slot_onCharacterAdded(SharedGroupChar character) void GroupWidget::slot_onCharacterRemoved(const GroupId characterId) { assert(characterId != INVALID_GROUPID); - m_model.removeCharacterById(characterId); + deref(m_model).removeCharacterById(characterId); updateColumnVisibility(); updatePulseTimer(); } @@ -1028,13 +1026,13 @@ void GroupWidget::slot_onCharacterRemoved(const GroupId characterId) void GroupWidget::slot_onCharacterUpdated(SharedGroupChar character) { assert(character); - m_model.updateCharacter(character); + deref(m_model).updateCharacter(character); updatePulseTimer(); } void GroupWidget::slot_onGroupReset(const GroupVector &newCharacterList) { - m_model.setCharacters(newCharacterList); + deref(m_model).setCharacters(newCharacterList); updateColumnVisibility(); updatePulseTimer(); } diff --git a/src/group/groupwidget.h b/src/group/groupwidget.h index 2f19fae25..5abe9db08 100644 --- a/src/group/groupwidget.h +++ b/src/group/groupwidget.h @@ -151,7 +151,7 @@ class NODISCARD_QOBJECT GroupWidget final : public QWidget Mmapper2Group *m_group = nullptr; MapData *m_map = nullptr; GroupProxyModel *m_proxyModel = nullptr; - GroupModel m_model; + GroupModel *m_model = nullptr; QTimer *m_pulseTimer = nullptr; void updateColumnVisibility(); @@ -174,8 +174,8 @@ class NODISCARD_QOBJECT GroupWidget final : public QWidget void sig_center(glm::vec2); public slots: - void slot_mapUnloaded() { m_model.setMapLoaded(false); } - void slot_mapLoaded() { m_model.setMapLoaded(true); } + void slot_mapUnloaded() { deref(m_model).setMapLoaded(false); } + void slot_mapLoaded() { deref(m_model).setMapLoaded(true); } private slots: void slot_onCharacterAdded(SharedGroupChar character); diff --git a/src/timers/TimerWidget.cpp b/src/timers/TimerWidget.cpp index e39d80d65..8a575da69 100644 --- a/src/timers/TimerWidget.cpp +++ b/src/timers/TimerWidget.cpp @@ -20,10 +20,10 @@ TimerWidget::TimerWidget(CTimers &timers, QWidget *parent) layout->setContentsMargins(0, 0, 0, 0); layout->setSpacing(0); - m_model = new TimerModel(m_timers, this); m_view = new QTableView(this); + m_model = new TimerModel(m_timers, m_view); m_view->setModel(m_model); - auto *delegate = new TimerDelegate(this); + auto *delegate = new TimerDelegate(m_view); for (int i = 0; i < TimerModel::ColCount; ++i) { m_view->setItemDelegateForColumn(i, delegate); }