Fix crash when grouping items in the layout designer by vicquick · Pull Request #65742 · qgis/QGIS

vicquick · 2026-04-08T18:13:31Z

Description

Reproduction

On master at ce4915f3:

Open a print layout and add two items (e.g. a north arrow and a label)
Select both in the items list, then click Group items
QGIS crashes with a segmentation fault from stack overflow

Root cause

QgsLayoutItemsListView::updateSelection() is connected to QItemSelectionModel::selectionChanged with a direct connection.

When a new group is created, QgsLayoutView::groupSelectedItems → QgsLayout::addLayoutItem → QgsLayoutModel::rebuildZList calls beginInsertRows, which shifts the persistent indices held by the items list view's selection model and causes QItemSelectionModel to emit selectionChanged synchronously — from inside the still-open beginInsertRows transaction.

updateSelection() then runs mid-transaction and calls QgsLayoutItem::setSelected() on items, which calls QgsLayoutModel::updateItemSelectStatus() and emits dataChanged on the source model. Emitting dataChanged from inside a beginInsertRows/endInsertRows bracket is a QAbstractItemModel contract violation.

On Qt 6.9.2 this cascades into infinite recursion:

groupSelectedItems
 → addLayoutItem → rebuildZList → beginInsertRows
   → persistent index shift → selectionChanged
     → updateSelection → setSelected → dataChanged
       → QTreeView::dataChanged touches selection model
         → selectionChanged → (recurses) → stack overflow (SIGSEGV)

The earlier manifestation reported in the issue thread — a duplicate row appearing for the newly created group in the items list that disappears after closing and reopening the layout — is the same root cause: the mid-transaction dataChanged corrupts QSortFilterProxyModel's internal source-to-proxy mapping cache and the new row ends up mapped twice. Rebuilding the proxy from scratch (by closing and reopening the layout) clears the duplicate.

Fix

Change the selectionChanged → updateSelection connection to Qt::QueuedConnection, so updateSelection() runs on the next event loop iteration — after endInsertRows has completed and the source model is in a consistent state.

Also add mUpdatingSelection to updateSelection()'s re-entry guard, matching the guard that onItemFocused() already uses. With the queued connection this is defense in depth, but it is cheap and protects against future refactors that might reintroduce a synchronous path into the slot.

Total diff: 1 file, +6 −3 lines.

Verification

Built master at ce4915f3 twice, unpatched and with this patch, on Ubuntu 25.10 + Qt 6.9.2:

Scenario	master (no fix)	master + this PR
2 items → group	SIGSEGV	clean
4 items → group	—	clean
Group then ungroup	—	clean
3 consecutive groupings	—	clean

AI tool usage

AI tool(s) (Copilot, Claude, or something similar) supported my development of this PR. See our policy about AI tool use. Use of AI tools must be indicated. Failure to be honest might result in banning.

Claude assisted with root cause analysis, fix implementation, and PR description drafting. The fix was verified by building QGIS from source and testing manually. I have reviewed and understand all code in this PR.

nyalldawson · 2026-04-08T22:01:56Z

Why did you delete the AI use disclaimer?

nyalldawson · 2026-04-08T22:04:36Z

Can you do a build with ENABLE_MODELTEST=TRUE, and then try to reproduce the bug with your fix included? That will flag if it introduces any new issues.

vicquick · 2026-04-09T16:32:54Z

Why did you delete the AI use disclaimer?

Hi, please excuse my oversight! That was unintentional, I indeed used AI to create this PR. I now updated the PR body with the disclaimer.

vicquick · 2026-04-09T16:34:44Z

Built at e63df2bf with -DENABLE_MODELTEST=TRUE and -DENABLE_TESTS=ON (Qt 6.9.2, GCC 15.2.0, Ubuntu 25.10).

Ran test_core_layoutmodel — all 25 tests pass, including groupSelection() and proxyCrash():

$ ./output/bin/test_core_layoutmodel
********* Start testing of TestQgsLayoutModel *********
Config: Using QtTest library 6.9.2, Qt 6.9.2 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 15.2.0), ubuntu 25.10
PASS   : TestQgsLayoutModel::initTestCase()
PASS   : TestQgsLayoutModel::creation()
PASS   : TestQgsLayoutModel::addItems()
PASS   : TestQgsLayoutModel::zList()
PASS   : TestQgsLayoutModel::clear()
PASS   : TestQgsLayoutModel::addLayoutItemDirectly()
PASS   : TestQgsLayoutModel::rebuildZList()
PASS   : TestQgsLayoutModel::removeItem()
PASS   : TestQgsLayoutModel::reorderUp()
PASS   : TestQgsLayoutModel::reorderDown()
PASS   : TestQgsLayoutModel::reorderTop()
PASS   : TestQgsLayoutModel::reorderBottom()
PASS   : TestQgsLayoutModel::moveItem()
PASS   : TestQgsLayoutModel::findItemAbove()
PASS   : TestQgsLayoutModel::findItemBelow()
PASS   : TestQgsLayoutModel::setItemRemoved()
PASS   : TestQgsLayoutModel::rebuildZListWithRemoved()
PASS   : TestQgsLayoutModel::reorderUpWithRemoved()
PASS   : TestQgsLayoutModel::reorderDownWithRemoved()
PASS   : TestQgsLayoutModel::reorderToTopWithRemoved()
PASS   : TestQgsLayoutModel::reorderToBottomWithRemoved()
PASS   : TestQgsLayoutModel::groupSelection()
PASS   : TestQgsLayoutModel::proxy()
PASS   : TestQgsLayoutModel::proxyCrash()
PASS   : TestQgsLayoutModel::cleanupTestCase()
Totals: 25 passed, 0 failed, 0 skipped, 0 blacklisted, 409ms
********* Finished testing of TestQgsLayoutModel *********

No model tester assertions. QAbstractItemModelTester runs in Fatal mode with ENABLE_MODELTEST — any contract violation (like the mid-transaction dataChanged this PR fixes) would crash the test immediately.

…te rows QgsLayoutItemsListView::updateSelection() was connected to QItemSelectionModel::selectionChanged with a direct connection. When the source QgsLayoutModel begins a row insertion (for example when creating a new group via QgsLayoutView::groupSelectedItems -> addLayoutItem -> rebuildZList -> beginInsertRows) and there are persistent indices held by the items list view's selection model, those indices shift and QItemSelectionModel emits selectionChanged synchronously, from inside the still-open beginInsertRows transaction. updateSelection() then runs while the source model is mid-transaction and calls setSelected() on layout items, which emits dataChanged() on the source model. Emitting dataChanged from inside a beginInsertRows/endInsertRows bracket is a QAbstractItemModel contract violation. The observable consequences differed by Qt version: - Qt 6.8: QTreeView::dataChanged eagerly touches the selection model, which re-emits selectionChanged, re-enters updateSelection(), and the cycle recurses until the stack overflows. - Qt 6.9+: The mid-transaction dataChanged corrupts QSortFilterProxyModel's internal source-to-proxy mapping cache. The newly inserted row ends up with two proxy rows mapping to the same source row, so the Items list panel visibly shows a duplicate entry for the newly created group. Closing and reopening the layout rebuilds the proxy from scratch and the duplicate disappears. Both symptoms share the same root cause: updateSelection() running synchronously inside an open source-model transaction. Making the selectionChanged -> updateSelection connection a Qt::QueuedConnection defers the slot to the next event loop iteration, by which time endInsertRows() has fired and the proxy is in a consistent state. The mUpdatingSelection re-entry guard is also added to updateSelection()'s early return, matching the guard that onItemFocused() already uses. With Qt::QueuedConnection this is defense in depth, but it is cheap and protects against future refactors that might reintroduce a synchronous path into the slot. Fixes qgis#61702 Assisted-by: Claude (Anthropic)

QgsMapLayerStyleManagerWidget::styleClicked() was connected to QAbstractItemView::clicked with a direct connection. When the user clicks a saved style in the Layer Styling panel's Style Manager tab, styleClicked() calls setCurrentStyle() synchronously from inside QListView::mouseReleaseEvent(). setCurrentStyle() emits currentStyleChanged(), which triggers currentStyleChanged() → mStyleList->setCurrentIndex(). This modifies the view's selection model while mouseReleaseEvent still holds a QPersistentModelIndex (d->pressedIndex). On Qt 6, the invalidated persistent index causes a use-after-free when mouseReleaseEvent continues after the clicked() handler returns. On Windows: access violation (0xbaadf00d = freed heap). On Linux: SIGSEGV → SIGABRT. The crash does NOT happen when switching styles via: - Right-click layer → Styles submenu (QAction::triggered fires from the menu event loop, outside any view event handler) - Layer Properties dialog (QComboBox::currentIndexChanged, also outside a view event handler) Fix: change the clicked → styleClicked connection to Qt::QueuedConnection, so styleClicked() runs on the next event loop iteration — after mouseReleaseEvent has fully completed and released d->pressedIndex. Same pattern as the layout group crash fixed in qgis#65742. Fixes qgis#65476 Assisted-by: Claude (Anthropic)

styleClicked() was connected to QAbstractItemView::clicked with a direct connection. When the user clicks a saved style in the Layer Styling panel's Style Manager tab, styleClicked() calls setCurrentStyle() synchronously from inside QListView::mouseReleaseEvent(). setCurrentStyle() emits currentStyleChanged(), which triggers currentStyleChanged() → mStyleList->setCurrentIndex(). This modifies the view's selection model while mouseReleaseEvent still holds a QPersistentModelIndex (d->pressedIndex). On Qt 6, the invalidated persistent index causes a use-after-free when mouseReleaseEvent continues after the clicked() handler returns. On Windows: access violation (0xbaadf00d = freed heap). On Linux: SIGSEGV → SIGABRT. The crash does NOT happen when switching styles via: - Right-click layer → Styles submenu (QAction::triggered fires from the menu event loop, outside any view event handler) - Layer Properties dialog (QComboBox::currentIndexChanged, also outside a view event handler) Fix: change the clicked → styleClicked connection to Qt::QueuedConnection, so styleClicked() runs on the next event loop iteration — after mouseReleaseEvent has fully completed and released d->pressedIndex. Same pattern as the layout group crash fixed in qgis#65742. Fixes qgis#65476 Assisted-by: Claude (Anthropic)

nyalldawson · 2026-04-09T23:35:38Z

@vicquick testqgslayoutmodel.cpp doesn't use the model test. You need to manually trigger the test by opening a layout designer window, and then grouping items. (Ie -- you can't rely on AI to do that test for you)

vicquick · 2026-04-10T22:10:19Z

@vicquick testqgslayoutmodel.cpp doesn't use the model test. You need to manually trigger the test by opening a layout designer window, and then grouping items. (Ie -- you can't rely on AI to do that test for you)

Understood, the automated tests do not cover that. Whilst I manually tested before, I have now rebuilt with -DENABLE_MODELTEST=TRUE, opened a layout, added several items, grouped/ungrouped several times — no crash, and no model warnings appeared, the logs stayed clean. On master without the fix, however, grouping crashes immediately with a stack overflow.

nyalldawson · 2026-04-10T22:40:19Z

Thanks for the confirmation @vicquick !

styleClicked() was connected to QAbstractItemView::clicked with a direct connection. When the user clicks a saved style in the Layer Styling panel's Style Manager tab, styleClicked() calls setCurrentStyle() synchronously from inside QListView::mouseReleaseEvent(). setCurrentStyle() emits currentStyleChanged(), which triggers currentStyleChanged() → mStyleList->setCurrentIndex(). This modifies the view's selection model while mouseReleaseEvent still holds a QPersistentModelIndex (d->pressedIndex). On Qt 6, the invalidated persistent index causes a use-after-free when mouseReleaseEvent continues after the clicked() handler returns. On Windows: access violation (0xbaadf00d = freed heap). On Linux: SIGSEGV → SIGABRT. The crash does NOT happen when switching styles via: - Right-click layer → Styles submenu (QAction::triggered fires from the menu event loop, outside any view event handler) - Layer Properties dialog (QComboBox::currentIndexChanged, also outside a view event handler) Fix: change the clicked → styleClicked connection to Qt::QueuedConnection, so styleClicked() runs on the next event loop iteration — after mouseReleaseEvent has fully completed and released d->pressedIndex. Same pattern as the layout group crash fixed in qgis#65742. Fixes qgis#65476 Assisted-by: Claude (Anthropic)

github-actions · 2026-04-11T00:19:23Z

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 51a789b)

🍎 MacOS Qt6 builds

Download MacOS Qt6 builds of this PR for testing.
This app is not notarized, run sudo xattr -d com.apple.quarantine /Applications/QGIS*.app to avoid the warning
(Built from commit 51a789b)

styleClicked() was connected to QAbstractItemView::clicked with a direct connection. When the user clicks a saved style in the Layer Styling panel's Style Manager tab, styleClicked() calls setCurrentStyle() synchronously from inside QListView::mouseReleaseEvent(). setCurrentStyle() emits currentStyleChanged(), which triggers currentStyleChanged() → mStyleList->setCurrentIndex(). This modifies the view's selection model while mouseReleaseEvent still holds a QPersistentModelIndex (d->pressedIndex). On Qt 6, the invalidated persistent index causes a use-after-free when mouseReleaseEvent continues after the clicked() handler returns. On Windows: access violation (0xbaadf00d = freed heap). On Linux: SIGSEGV → SIGABRT. The crash does NOT happen when switching styles via: - Right-click layer → Styles submenu (QAction::triggered fires from the menu event loop, outside any view event handler) - Layer Properties dialog (QComboBox::currentIndexChanged, also outside a view event handler) Fix: change the clicked → styleClicked connection to Qt::QueuedConnection, so styleClicked() runs on the next event loop iteration — after mouseReleaseEvent has fully completed and released d->pressedIndex. Same pattern as the layout group crash fixed in qgis#65742. Fixes qgis#65476 Assisted-by: Claude (Anthropic)

github-actions · 2026-04-23T03:43:36Z

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has been marked as AI written and has not had any activity in the last 5 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that you have personally verified that the AI driven changes work, fit within
existing QGIS architecture and design, and are well-written.
that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular
- link to any issues which this pull request fixes
- add a description of workflows which this pull request fixes
- add screenshots if applicable
that you have written unit tests where possible
If there is no further activity on this pull request, it will be closed in 5 days.

vicquick mentioned this pull request Apr 8, 2026

Grouping two items in layout viewer and QGIS crashed. #61702

Open

2 tasks

github-actions Bot added the Print Layouts Related to QGIS Print Layouts, Atlas or Reporting frameworks label Apr 8, 2026

github-actions Bot added this to the 4.2.0 milestone Apr 8, 2026

vicquick force-pushed the fix/61702-layout-items-list-queued-connection branch from e63df2b to 51a789b Compare April 9, 2026 16:35

nyalldawson added the AI assisted⚠️ AI assisted coding involved. Review with extreme scepticism label Apr 19, 2026

github-actions Bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Apr 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix crash when grouping items in the layout designer#65742

Fix crash when grouping items in the layout designer#65742
vicquick wants to merge 1 commit intoqgis:masterfrom
vicquick:fix/61702-layout-items-list-queued-connection

vicquick commented Apr 8, 2026 •

edited

Loading

Uh oh!

nyalldawson commented Apr 8, 2026

Uh oh!

nyalldawson commented Apr 8, 2026

Uh oh!

vicquick commented Apr 9, 2026

Uh oh!

vicquick commented Apr 9, 2026

Uh oh!

nyalldawson commented Apr 9, 2026

Uh oh!

vicquick commented Apr 10, 2026

Uh oh!

nyalldawson commented Apr 10, 2026

Uh oh!

github-actions Bot commented Apr 11, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

vicquick commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reproduction

Root cause

Fix

Verification

AI tool usage

Uh oh!

nyalldawson commented Apr 8, 2026

Uh oh!

nyalldawson commented Apr 8, 2026

Uh oh!

vicquick commented Apr 9, 2026

Uh oh!

vicquick commented Apr 9, 2026

Uh oh!

nyalldawson commented Apr 9, 2026

Uh oh!

vicquick commented Apr 10, 2026

Uh oh!

nyalldawson commented Apr 10, 2026

Uh oh!

github-actions Bot commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🪟 Windows Qt6 builds

🍎 MacOS Qt6 builds

Uh oh!

github-actions Bot commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vicquick commented Apr 8, 2026 •

edited

Loading

github-actions Bot commented Apr 11, 2026 •

edited

Loading