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.
This commit is contained in:
ispyisail
2026-08-05 08:29:16 +12:00
parent f6b448daa7
commit f83aa3f1bc
@@ -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()) {