Skip to content

Commit e43ffdc

Browse files
committed
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.
1 parent 92cd4e3 commit e43ffdc

3 files changed

Lines changed: 22 additions & 24 deletions

File tree

src/group/groupwidget.cpp

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -829,14 +829,7 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget
829829
: QWidget(parent)
830830
, m_group(group)
831831
, m_map(md)
832-
, m_model(this)
833832
{
834-
if (m_group) {
835-
m_model.setCharacters(m_group->selectAll());
836-
} else {
837-
m_model.setCharacters({});
838-
}
839-
840833
auto *layout = new QVBoxLayout(this);
841834
layout->setAlignment(Qt::AlignTop);
842835
layout->setContentsMargins(0, 0, 0, 0);
@@ -849,8 +842,15 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget
849842
m_table->horizontalHeader()->setStretchLastSection(true);
850843
m_table->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents);
851844

852-
m_proxyModel = new GroupProxyModel(this);
853-
m_proxyModel->setSourceModel(&m_model);
845+
m_model = new GroupModel(m_table);
846+
if (m_group) {
847+
m_model->setCharacters(m_group->selectAll());
848+
} else {
849+
m_model->setCharacters({});
850+
}
851+
852+
m_proxyModel = new GroupProxyModel(m_table);
853+
m_proxyModel->setSourceModel(m_model);
854854
m_table->setModel(m_proxyModel);
855855

856856
m_table->setDragEnabled(true);
@@ -859,7 +859,7 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget
859859
m_table->setDefaultDropAction(Qt::MoveAction);
860860
m_table->setDropIndicatorShown(true);
861861

862-
m_table->setItemDelegate(new GroupDelegate(this));
862+
m_table->setItemDelegate(new GroupDelegate(m_table));
863863
layout->addWidget(m_table);
864864

865865
m_pulseTimer = new QTimer(this);
@@ -912,7 +912,7 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget
912912
return;
913913
}
914914

915-
selectedCharacter = m_model.getCharacter(sourceIndex.row());
915+
selectedCharacter = deref(m_model).getCharacter(sourceIndex.row());
916916
if (selectedCharacter) {
917917
// Build Context menu
918918
m_center->setText(
@@ -947,8 +947,6 @@ GroupWidget::GroupWidget(Mmapper2Group *const group, MapData *const md, QWidget
947947
GroupWidget::~GroupWidget()
948948
{
949949
m_pulseTimer->stop();
950-
delete m_table;
951-
delete m_recolor;
952950
}
953951

954952
QSize GroupWidget::sizeHint() const
@@ -964,7 +962,7 @@ void GroupWidget::updateColumnVisibility()
964962
{
965963
// Hide unnecessary columns like mana if everyone is a zorc/troll
966964
const auto one_character_had_mana = [this]() -> bool {
967-
for (const auto &character : m_model.getCharacters()) {
965+
for (const auto &character : deref(m_model).getCharacters()) {
968966
if (character && (character->getMana() > 0 || character->getMaxMana() > 0)) {
969967
return true;
970968
}
@@ -978,7 +976,7 @@ void GroupWidget::updateColumnVisibility()
978976
void GroupWidget::updatePulseTimer()
979977
{
980978
const auto needs_pulse = [this]() -> bool {
981-
for (const auto &character : m_model.getCharacters()) {
979+
for (const auto &character : deref(m_model).getCharacters()) {
982980
if (!character) {
983981
continue;
984982
}
@@ -1012,29 +1010,29 @@ void GroupWidget::updatePulseTimer()
10121010
void GroupWidget::slot_onCharacterAdded(SharedGroupChar character)
10131011
{
10141012
assert(character);
1015-
m_model.insertCharacter(character);
1013+
deref(m_model).insertCharacter(character);
10161014
updateColumnVisibility();
10171015
updatePulseTimer();
10181016
}
10191017

10201018
void GroupWidget::slot_onCharacterRemoved(const GroupId characterId)
10211019
{
10221020
assert(characterId != INVALID_GROUPID);
1023-
m_model.removeCharacterById(characterId);
1021+
deref(m_model).removeCharacterById(characterId);
10241022
updateColumnVisibility();
10251023
updatePulseTimer();
10261024
}
10271025

10281026
void GroupWidget::slot_onCharacterUpdated(SharedGroupChar character)
10291027
{
10301028
assert(character);
1031-
m_model.updateCharacter(character);
1029+
deref(m_model).updateCharacter(character);
10321030
updatePulseTimer();
10331031
}
10341032

10351033
void GroupWidget::slot_onGroupReset(const GroupVector &newCharacterList)
10361034
{
1037-
m_model.setCharacters(newCharacterList);
1035+
deref(m_model).setCharacters(newCharacterList);
10381036
updateColumnVisibility();
10391037
updatePulseTimer();
10401038
}

src/group/groupwidget.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ class NODISCARD_QOBJECT GroupWidget final : public QWidget
151151
Mmapper2Group *m_group = nullptr;
152152
MapData *m_map = nullptr;
153153
GroupProxyModel *m_proxyModel = nullptr;
154-
GroupModel m_model;
154+
GroupModel *m_model = nullptr;
155155
QTimer *m_pulseTimer = nullptr;
156156

157157
void updateColumnVisibility();
@@ -174,8 +174,8 @@ class NODISCARD_QOBJECT GroupWidget final : public QWidget
174174
void sig_center(glm::vec2);
175175

176176
public slots:
177-
void slot_mapUnloaded() { m_model.setMapLoaded(false); }
178-
void slot_mapLoaded() { m_model.setMapLoaded(true); }
177+
void slot_mapUnloaded() { deref(m_model).setMapLoaded(false); }
178+
void slot_mapLoaded() { deref(m_model).setMapLoaded(true); }
179179

180180
private slots:
181181
void slot_onCharacterAdded(SharedGroupChar character);

src/timers/TimerWidget.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ TimerWidget::TimerWidget(CTimers &timers, QWidget *parent)
2020
layout->setContentsMargins(0, 0, 0, 0);
2121
layout->setSpacing(0);
2222

23-
m_model = new TimerModel(m_timers, this);
2423
m_view = new QTableView(this);
24+
m_model = new TimerModel(m_timers, m_view);
2525
m_view->setModel(m_model);
26-
auto *delegate = new TimerDelegate(this);
26+
auto *delegate = new TimerDelegate(m_view);
2727
for (int i = 0; i < TimerModel::ColCount; ++i) {
2828
m_view->setItemDelegateForColumn(i, delegate);
2929
}

0 commit comments

Comments
 (0)