From f83aa3f1bc18823e6b9ab6a93cf320b09f58d916 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Wed, 5 Aug 2026 08:29:16 +1200 Subject: [PATCH] Fix stack-overflow crash in FileElementCollectionItem::setUpIcon() The unconditional early return was narrowed to non-directories only, so that the just-added warning badge could be picked up once setUpData() resolved m_qet_directory_unreadable asynchronously. But every directory then called setIcon() on every single data(Qt::DecorationRole) query -- not just once -- and QStandardItem::setIcon() -> setData() emits dataChanged() unconditionally (QIcon has no equality check to suppress it). QTreeView handles dataChanged() by recomputing the row's size hint, which re-enters data() for the same index, calling setIcon() again: unbounded mutual recursion, confirmed by an isolated reproduction to overflow the stack in a single frame (100k+ frames) well before the first paint completes. Matches plc-user's report of a segfault right as the elements tree begins drawing. The race the guard was widened for doesn't actually occur: ElementsCollectionModel only attaches itself to the tree view (the only way data() becomes reachable) from loadingFinished(), which fires after the QtConcurrent::map over every item -- this one included -- has already finished. m_qet_directory_unreadable is therefore always final before setUpIcon() can run for the first time, so the plain, always-only- once guard is sufficient and the badge still works correctly. --- .../fileelementcollectionitem.cpp | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/sources/ElementsCollection/fileelementcollectionitem.cpp b/sources/ElementsCollection/fileelementcollectionitem.cpp index ccd58d0b9..5e5c7e844 100644 --- a/sources/ElementsCollection/fileelementcollectionitem.cpp +++ b/sources/ElementsCollection/fileelementcollectionitem.cpp @@ -430,13 +430,20 @@ void FileElementCollectionItem::setUpData() */ void FileElementCollectionItem::setUpIcon() { - // Directories are cheap to (re-)decide: no early return for them. - // setUpData() -- which resolves m_qet_directory_unreadable via - // localName() -- runs asynchronously (QtConcurrent::map), so this can - // be called for a directory before that result is known; without - // this, the plain folder icon would get cached by the guard below - // and the warning badge would never appear for that item. - if (!isDir() && !icon().isNull()) + // Must return unconditionally once an icon is set: setIcon() calls + // setData(), which emits dataChanged() regardless of whether the new + // icon differs from the old one (QIcon has no meaningful equality). + // QTreeView responds to dataChanged() by recomputing the row's size + // hint, which re-enters data() for this same index -- so without this + // guard, any repeated setIcon() here recurses until the stack + // overflows. Confirmed by crash report on PR #633. + // + // This item's m_qet_directory_unreadable is already final by the time + // this can run at all: ElementsCollectionModel only attaches itself + // to the tree view (making data() reachable) from loadingFinished(), + // which fires after the QtConcurrent::map over every item -- this one + // included -- has completed. So there is no race to work around here. + if (!icon().isNull()) return; if (isCollectionRoot()) {