Skip to content

Fix mavlink inspector crash #13309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Fix mavlink inspector crash #13309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3bb9656
Fix mavlink inspector crash
vryabokon1705 Aug 15, 2025
c52d7de
git-ignore back
vryabokon1705 Aug 15, 2025
cf194fa
Iindentation
vryabokon1705 Aug 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/AnalyzeView/MAVLinkInspectorController.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,12 @@ MAVLinkInspectorController::~MAVLinkInspectorController()
{
qDeleteAll(_timeScaleSt);
qDeleteAll(_rangeSt);
_charts->clearAndDeleteContents();

for (int i = _charts->count() - 1; i >= 0; i--) {
auto chartObject = _charts->get(i);
MAVLinkChartController* chart = dynamic_cast<MAVLinkChartController*>(chartObject);
deleteChart(chart);
}
Copy link
Contributor

@DonLakeFlyer DonLakeFlyer Sep 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how this fixes a problem? As far as I can tell it ends up being functionally the same thing. The only diiference is that chartsChanged is not signalled. But given the fact this is happening during destruction that should be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It helps to prevent trying to draw the chart, which was already deleted, but the list has not been clear yet

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How, given the fact that it is functionally equivalent to the previous code? Is it related to the chartsChanged signalling?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

void QmlObjectListModel::clearAndDeleteContents()
{
    for (int i=0; i<_objectList.count(); i++) {
        _objectList[i]->deleteLater();
    }
    clear();
}

In this method, we first iterate over the list and delete objects, but pointers still exist. And during this process, before _objectList is cleared, we can try to draw the chart, which can lead to a null pointer exception

_systems->clearAndDeleteContents();

// qCDebug(MAVLinkInspectorControllerLog) << Q_FUNC_INFO << this;
Expand Down Expand Up @@ -227,9 +232,9 @@ void MAVLinkInspectorController::_receiveMessage(LinkInterface *link, const mavl
}
}

MAVLinkChartController *MAVLinkInspectorController::createChart()
MAVLinkChartController *MAVLinkInspectorController::createChart(int chartIndex)
{
MAVLinkChartController *const pChart = new MAVLinkChartController(this, _charts->count());
MAVLinkChartController *const pChart = new MAVLinkChartController(this, chartIndex);
QQmlEngine::setObjectOwnership(pChart, QQmlEngine::CppOwnership);

_charts->append(pChart);
Expand Down
2 changes: 1 addition & 1 deletion src/AnalyzeView/MAVLinkInspectorController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class MAVLinkInspectorController : public QObject
explicit MAVLinkInspectorController(QObject *parent = nullptr);
~MAVLinkInspectorController();

Q_INVOKABLE MAVLinkChartController *createChart();
Q_INVOKABLE MAVLinkChartController *createChart(int chartIndex);
Q_INVOKABLE void deleteChart(MAVLinkChartController *chart);
Q_INVOKABLE void setActiveSystem(int systemId);
Q_INVOKABLE void setMessageInterval(int32_t rate) const;
Expand Down
8 changes: 4 additions & 4 deletions src/AnalyzeView/MAVLinkInspectorPage.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,9 @@ AnalyzePage {
checked: object.series !== null && object.chartIndex === 0
onClicked: {
if(checked) {
chart1.addDimension(object)
chart1.addDimension(object, 0)
} else {
chart1.delDimension(object)
chart1.delDimension(object, 0)
}
updateEnabledStatus(chart1Repeater, curMessage, chart1)
updateEnabledStatus(chart2Repeater, curMessage, chart2)
Expand All @@ -325,9 +325,9 @@ AnalyzePage {
checked: object.series !== null && object.chartIndex === 1
onClicked: {
if(checked) {
chart2.addDimension(object)
chart2.addDimension(object, 1)
} else {
chart2.delDimension(object)
chart2.delDimension(object, 1)
}
updateEnabledStatus(chart2Repeater, curMessage, chart2)
updateEnabledStatus(chart1Repeater, curMessage, chart1)
Expand Down
4 changes: 2 additions & 2 deletions src/QmlControls/MAVLinkChart.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ ChartView {
property var chartController: null
property var seriesColors: ["#00E04B","#DE8500","#F32836","#BFBFBF","#536DFF","#EECC44"]

function addDimension(field) {
function addDimension(field, chartIndex) {
if(!chartController) {
chartController = controller.createChart()
chartController = controller.createChart(chartIndex)
}
var color = chartView.seriesColors[chartView.count]
var serie = createSeries(ChartView.SeriesTypeLine, field.label)
Expand Down
Loading