Fix double-removal Q_ASSERT in DockManagerPrivate::restoreStateFromXml#838
Open
fdarmayan wants to merge 1 commit into
Open
Fix double-removal Q_ASSERT in DockManagerPrivate::restoreStateFromXml#838fdarmayan wants to merge 1 commit into
fdarmayan wants to merge 1 commit into
Conversation
In DockManagerPrivate::restoreStateFromXml() the floating widgets were removed via removeDockContainer() and then queued for deletion with deleteLater(). When ~CDockContainerWidget() ran it would call removeDockContainer() a second time on the same container, tripping the Q_ASSERT(removed == 1) check in CDockManager::removeDockContainer(). Switch to CDockContainerWidget::removeFromDockManager() (introduced in upstream 544c624) so the container's back-pointer to the manager is cleared up front and the destructor becomes a no-op for the manager side. This keeps the LP360_AI workspace restore path crash-free until the equivalent fix lands upstream.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CDockManager::restoreState()can crash with a failed assertion when thesaved state contains floating dock containers. The teardown path in
DockManagerPrivate::restoreStateFromXml()looks like:removeDockContainer()is called and the floating widget is queued fordeletion. When the queued
deleteLater()fires,~CDockContainerWidget()runs and calls
removeDockContainer()on the manager a second time forthe same container. That second call trips:
inside
CDockManager::removeDockContainer(), killing the application indebug builds and silently corrupting the container list in release.
Fix
Use
CDockContainerWidget::removeFromDockManager()(introduced in544c624 "Unbind containers from DockManager to prevent accidental reuse
before deletion") instead of the manager-side
removeDockContainer().That helper clears the container's back-pointer to the manager up front,
so the subsequent destructor call becomes a no-op for the manager and
the assertion no longer fires.
Repro
CDockManagerwith at least one floating dockcontainer in the saved layout.
CDockManager::saveState()/restoreState()on that layoutwhile floating widgets exist.
Q_ASSERT(removed == 1)failure inCDockManager::removeDockContainer()oncedeleteLater()drains.Notes
removeFromDockManager()already does thesame list-removal work that
removeDockContainer()did, just withoutthe double-removal hazard.