From a45ca338483106a1328bc4031e6a42bd456802cc Mon Sep 17 00:00:00 2001 From: sbvis Date: Fri, 3 Oct 2025 00:34:24 +0200 Subject: [PATCH 1/5] fix for macos scatterplot crash --- src/ColoringAction.cpp | 23 +++++++++++++++++++++++ src/ColoringAction.h | 5 +++++ src/ScatterplotPlugin.cpp | 26 ++++++++++++++++++++++++++ src/SettingsAction.cpp | 22 ++++++++++++++++++++++ src/SettingsAction.h | 5 +++++ 5 files changed, 81 insertions(+) diff --git a/src/ColoringAction.cpp b/src/ColoringAction.cpp index c3fca1f..88b693d 100644 --- a/src/ColoringAction.cpp +++ b/src/ColoringAction.cpp @@ -164,6 +164,29 @@ ColoringAction::ColoringAction(QObject* parent, const QString& title) : _scatterplotPlugin->getScatterplotWidget().setColoringMode(ScatterplotWidget::ColoringMode::Constant); } +ColoringAction::~ColoringAction() +{ + // Disconnect all connections to prevent accessing freed memory during destruction + if (_scatterplotPlugin) { + disconnect(&_scatterplotPlugin->getPositionDataset(), nullptr, this, nullptr); + disconnect(&_scatterplotPlugin->getPositionSourceDataset(), nullptr, this, nullptr); + disconnect(&_scatterplotPlugin->getScatterplotWidget(), nullptr, this, nullptr); + } + + // Disconnect child action connections + disconnect(&_colorByAction, nullptr, this, nullptr); + disconnect(&_colorByModel, nullptr, this, nullptr); + disconnect(&_dimensionAction, nullptr, this, nullptr); + disconnect(&_constantColorAction, nullptr, this, nullptr); + disconnect(&_colorMap1DAction, nullptr, this, nullptr); + disconnect(&_colorMap2DAction, nullptr, this, nullptr); + + // Disconnect current dataset if valid + if (_currentColorPointsDataset.isValid()) { + disconnect(&_currentColorPointsDataset, nullptr, this, nullptr); + } +} + QMenu* ColoringAction::getContextMenu(QWidget* parent /*= nullptr*/) { auto menu = new QMenu("Color", parent); diff --git a/src/ColoringAction.h b/src/ColoringAction.h index 4a1d480..6f5eacf 100644 --- a/src/ColoringAction.h +++ b/src/ColoringAction.h @@ -35,6 +35,11 @@ class ColoringAction : public VerticalGroupAction * @param title Title */ Q_INVOKABLE ColoringAction(QObject* parent, const QString& title); + + /** + * Destructor - ensures proper cleanup of Qt connections + */ + ~ColoringAction(); /** * Get the context menu for the action diff --git a/src/ScatterplotPlugin.cpp b/src/ScatterplotPlugin.cpp index 1e35b7c..5c0d197 100644 --- a/src/ScatterplotPlugin.cpp +++ b/src/ScatterplotPlugin.cpp @@ -292,6 +292,29 @@ ScatterplotPlugin::ScatterplotPlugin(const PluginFactory* factory) : ScatterplotPlugin::~ScatterplotPlugin() { + // Ensure proper cleanup order - disconnect any remaining connections + // before member variables are destroyed + if (_scatterPlotWidget) { + disconnect(_scatterPlotWidget, nullptr, this, nullptr); + disconnect(&_scatterPlotWidget->getPixelSelectionTool(), nullptr, this, nullptr); + } + + // Clear any remaining connections from position dataset + if (_positionDataset.isValid()) { + disconnect(&_positionDataset, nullptr, this, nullptr); + } + + // Disconnect all connections from settings action members to avoid + // accessing freed memory during member destruction + disconnect(&_settingsAction, nullptr, this, nullptr); + disconnect(&_settingsAction.getColoringAction(), nullptr, this, nullptr); + disconnect(&_settingsAction.getColoringAction().getColorByAction(), nullptr, this, nullptr); + disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getFocusSelection(), nullptr, this, nullptr); + disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getSizeAction(), nullptr, this, nullptr); + disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getOpacityAction(), nullptr, this, nullptr); + + // Disconnect from sampler action + disconnect(&getSamplerAction(), nullptr, this, nullptr); } void ScatterplotPlugin::init() @@ -1017,6 +1040,9 @@ void ScatterplotPlugin::updateSelection() void ScatterplotPlugin::updateHeadsUpDisplay() { +#ifdef __APPLE__ + return; +#endif getHeadsUpDisplayAction().removeAllHeadsUpDisplayItems(); if (_positionDataset.isValid()) { diff --git a/src/SettingsAction.cpp b/src/SettingsAction.cpp index 03d0f82..8b63e66 100644 --- a/src/SettingsAction.cpp +++ b/src/SettingsAction.cpp @@ -45,6 +45,28 @@ SettingsAction::SettingsAction(QObject* parent, const QString& title) : connect(&_scatterplotPlugin->getPositionDataset(), &Dataset::changed, this, updateEnabled); } +SettingsAction::~SettingsAction() +{ + // Disconnect all connections to prevent accessing freed memory during destruction + if (_scatterplotPlugin) { + disconnect(&_scatterplotPlugin->getPositionDataset(), nullptr, this, nullptr); + disconnect(&_scatterplotPlugin->getPositionSourceDataset(), nullptr, this, nullptr); + disconnect(&_scatterplotPlugin->getScatterplotWidget(), nullptr, this, nullptr); + } + + // Disconnect all child actions to prevent cross-references during destruction + disconnect(&_coloringAction, nullptr, this, nullptr); + disconnect(&_plotAction, nullptr, this, nullptr); + disconnect(&_positionAction, nullptr, this, nullptr); + disconnect(&_renderModeAction, nullptr, this, nullptr); + disconnect(&_selectionAction, nullptr, this, nullptr); + disconnect(&_subsetAction, nullptr, this, nullptr); + disconnect(&_clusteringAction, nullptr, this, nullptr); + disconnect(&_exportAction, nullptr, this, nullptr); + disconnect(&_miscellaneousAction, nullptr, this, nullptr); + disconnect(&_datasetsAction, nullptr, this, nullptr); +} + QMenu* SettingsAction::getContextMenu() { auto menu = new QMenu(); diff --git a/src/SettingsAction.h b/src/SettingsAction.h index a6b0ba9..726b23e 100644 --- a/src/SettingsAction.h +++ b/src/SettingsAction.h @@ -34,6 +34,11 @@ class SettingsAction : public GroupAction * @param title Title */ Q_INVOKABLE SettingsAction(QObject* parent, const QString& title); + + /** + * Destructor - ensures proper cleanup of Qt connections + */ + ~SettingsAction(); /** * Get action context menu From 47686e43c9548f9df8934ed1f8d61f06667f2eb9 Mon Sep 17 00:00:00 2001 From: sbvis Date: Fri, 3 Oct 2025 21:44:08 +0200 Subject: [PATCH 2/5] memory corruption crash fix --- .DS_Store | Bin 0 -> 6148 bytes src/ScatterplotPlugin.cpp | 11 +++++++++++ 2 files changed, 11 insertions(+) create mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..5418c828a296a17ebff3f1e4c56aa9d457042c6f GIT binary patch literal 6148 zcmeHK%}T>S5T0!#M)Xh-Q9SJfsL(fvHB#w8guZ~Jg;r=uQG3s8co6Ttg%^Dh@#0y} zezQBaSz|q_$PCPWoB7$9d`YuiA~LOMze&^}q8Q3pTfy*!u%EReHLw|`upGdC&SXav2%)4 zTK=d@Lz+?+RHw#uHKN=@`|0lC_R#h4X`OLC?__aQ&<|Per%so2LA{`lX!C9EDc9e4 z&R%ZXuD;}z>sGx%y(RsLFySi7fHLrF3}DY@iB~=9s0=6r%D{{P{ytbJV9AXgO9Ts#KAnslK8#d0azZh#I@Y%? zoJ`_TM`b`62pOpR%LecNM~lz@VUk`c1Ioa^V!%XcH*MjTWN)q99PhOjdIn|TxWeNs j1rxUwBbK+~ZKxC2El+@nVBrxKi2f09H0YoV{3rwOfF5Js literal 0 HcmV?d00001 diff --git a/src/ScatterplotPlugin.cpp b/src/ScatterplotPlugin.cpp index 5c0d197..5e9427b 100644 --- a/src/ScatterplotPlugin.cpp +++ b/src/ScatterplotPlugin.cpp @@ -297,6 +297,10 @@ ScatterplotPlugin::~ScatterplotPlugin() if (_scatterPlotWidget) { disconnect(_scatterPlotWidget, nullptr, this, nullptr); disconnect(&_scatterPlotWidget->getPixelSelectionTool(), nullptr, this, nullptr); + // Remove from parent to prevent double-deletion by Qt's cleanup + _scatterPlotWidget->setParent(nullptr); + delete _scatterPlotWidget; + _scatterPlotWidget = nullptr; } // Clear any remaining connections from position dataset @@ -315,6 +319,13 @@ ScatterplotPlugin::~ScatterplotPlugin() // Disconnect from sampler action disconnect(&getSamplerAction(), nullptr, this, nullptr); + + // Clean up drop widget after scatterplot widget to maintain proper order + if (_dropWidget) { + _dropWidget->setParent(nullptr); + delete _dropWidget; + _dropWidget = nullptr; + } } void ScatterplotPlugin::init() From 08513c30cbb76618879076c2bee8c87e547f634c Mon Sep 17 00:00:00 2001 From: Soumyadeep Basu <44787782+sbvis@users.noreply.github.com> Date: Fri, 3 Oct 2025 21:44:58 +0200 Subject: [PATCH 3/5] Delete .DS_Store --- .DS_Store | Bin 6148 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store deleted file mode 100644 index 5418c828a296a17ebff3f1e4c56aa9d457042c6f..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHK%}T>S5T0!#M)Xh-Q9SJfsL(fvHB#w8guZ~Jg;r=uQG3s8co6Ttg%^Dh@#0y} zezQBaSz|q_$PCPWoB7$9d`YuiA~LOMze&^}q8Q3pTfy*!u%EReHLw|`upGdC&SXav2%)4 zTK=d@Lz+?+RHw#uHKN=@`|0lC_R#h4X`OLC?__aQ&<|Per%so2LA{`lX!C9EDc9e4 z&R%ZXuD;}z>sGx%y(RsLFySi7fHLrF3}DY@iB~=9s0=6r%D{{P{ytbJV9AXgO9Ts#KAnslK8#d0azZh#I@Y%? zoJ`_TM`b`62pOpR%LecNM~lz@VUk`c1Ioa^V!%XcH*MjTWN)q99PhOjdIn|TxWeNs j1rxUwBbK+~ZKxC2El+@nVBrxKi2f09H0YoV{3rwOfF5Js From e6e3b5df7843efeacbf13c47f8bcc22fb42f8980 Mon Sep 17 00:00:00 2001 From: sbvis Date: Sat, 4 Oct 2025 23:34:44 +0200 Subject: [PATCH 4/5] Removed manual widget deletion --- src/ScatterplotPlugin.cpp | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/src/ScatterplotPlugin.cpp b/src/ScatterplotPlugin.cpp index 5e9427b..8f2894e 100644 --- a/src/ScatterplotPlugin.cpp +++ b/src/ScatterplotPlugin.cpp @@ -292,40 +292,9 @@ ScatterplotPlugin::ScatterplotPlugin(const PluginFactory* factory) : ScatterplotPlugin::~ScatterplotPlugin() { - // Ensure proper cleanup order - disconnect any remaining connections - // before member variables are destroyed - if (_scatterPlotWidget) { - disconnect(_scatterPlotWidget, nullptr, this, nullptr); - disconnect(&_scatterPlotWidget->getPixelSelectionTool(), nullptr, this, nullptr); - // Remove from parent to prevent double-deletion by Qt's cleanup - _scatterPlotWidget->setParent(nullptr); - delete _scatterPlotWidget; - _scatterPlotWidget = nullptr; - } - - // Clear any remaining connections from position dataset if (_positionDataset.isValid()) { disconnect(&_positionDataset, nullptr, this, nullptr); } - - // Disconnect all connections from settings action members to avoid - // accessing freed memory during member destruction - disconnect(&_settingsAction, nullptr, this, nullptr); - disconnect(&_settingsAction.getColoringAction(), nullptr, this, nullptr); - disconnect(&_settingsAction.getColoringAction().getColorByAction(), nullptr, this, nullptr); - disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getFocusSelection(), nullptr, this, nullptr); - disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getSizeAction(), nullptr, this, nullptr); - disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getOpacityAction(), nullptr, this, nullptr); - - // Disconnect from sampler action - disconnect(&getSamplerAction(), nullptr, this, nullptr); - - // Clean up drop widget after scatterplot widget to maintain proper order - if (_dropWidget) { - _dropWidget->setParent(nullptr); - delete _dropWidget; - _dropWidget = nullptr; - } } void ScatterplotPlugin::init() From 74c0a6dcfb599e150730a9856308433c38362e3c Mon Sep 17 00:00:00 2001 From: sbvis Date: Sun, 5 Oct 2025 00:44:48 +0200 Subject: [PATCH 5/5] no signal callbacks can fire on the ScatterplotPlugin object while it's being destroyed, preventing the memory corruption that was causing the crash --- src/ScatterplotPlugin.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/ScatterplotPlugin.cpp b/src/ScatterplotPlugin.cpp index 8f2894e..a9f67b0 100644 --- a/src/ScatterplotPlugin.cpp +++ b/src/ScatterplotPlugin.cpp @@ -292,9 +292,27 @@ ScatterplotPlugin::ScatterplotPlugin(const PluginFactory* factory) : ScatterplotPlugin::~ScatterplotPlugin() { + disconnect(this); if (_positionDataset.isValid()) { disconnect(&_positionDataset, nullptr, this, nullptr); } + + if (_scatterPlotWidget) { + disconnect(_scatterPlotWidget, nullptr, this, nullptr); + disconnect(&_scatterPlotWidget->getPixelSelectionTool(), nullptr, this, nullptr); + disconnect(&_scatterPlotWidget->getPointRendererNavigator().getNavigationAction().getZoomSelectionAction(), nullptr, this, nullptr); + } + disconnect(&_settingsAction, nullptr, this, nullptr); + disconnect(&_settingsAction.getColoringAction(), nullptr, this, nullptr); + disconnect(&_settingsAction.getColoringAction().getColorByAction(), nullptr, this, nullptr); + disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getFocusSelection(), nullptr, this, nullptr); + disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getSizeAction(), nullptr, this, nullptr); + disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getOpacityAction(), nullptr, this, nullptr); + disconnect(&_settingsAction.getColoringAction(), &ColoringAction::currentColorDatasetChanged, this, &ScatterplotPlugin::updateHeadsUpDisplay); + disconnect(&_settingsAction.getColoringAction().getColorByAction(), &OptionAction::currentIndexChanged, this, &ScatterplotPlugin::updateHeadsUpDisplay); + disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getSizeAction(), &ScalarAction::sourceDataChanged, this, &ScatterplotPlugin::updateHeadsUpDisplay); + disconnect(&_settingsAction.getPlotAction().getPointPlotAction().getOpacityAction(), &ScalarAction::sourceDataChanged, this, &ScatterplotPlugin::updateHeadsUpDisplay); + disconnect(&getSamplerAction(), nullptr, this, nullptr); } void ScatterplotPlugin::init()