Fix crash from selected nodes when closing scene #108785
Conversation
Fixes godotengine#108783 In a recent change, the `SceneTreedock` node started keeping track of selected nodes so it could disconnect some signals from them whenever the selection changed. However, when a scene is closed, its nodes are deleted before the selection change can complete, causing invalid access to dangling pointers. To fix it, the editor now explicitly tells the scene tree dock to clean after itself when the currently edited scene is closing. Rationale for implementation decisions: 1. I added and called a method on `SceneTreeDock` (as opposed to using a signal) because `EditorNode` already calls many methods directly on the dock. 2. I call the new method on `EditorNode::_remove_edited_scene()` since it seems to be the most specific to the situation at hand. Simply closing any scene does not cause the problem, since tracked nodes come from the selection, which applies only to the currently edited scene.
|
@Kazuo256 verified locally, no crash now, Thank you! |
Maybe we should fix that instead? It means that at some point the editor selection is invalid. The nodes should be deselected before being deleted. Although the current fix is safer and simpler I suppose. |
|
My position is that since this is a very glaring crash and we are on the beta phase of the release cycle, we should first fix it fast and simple. I see there are some other PRs trying to fix the same thing in a similar way, any would do to me. At the same time, we should open an issue describing the underlying problem so it doesn't get lost. Either way, it would make more sense to open a new PR to fix that. |
|
Ok, after checking the code, the problem is that clearing the selection is deferred. |
|
Superseded by #108883 |
Fixes #108783
In a recent change, the
SceneTreedocknode started keeping track of selected nodes so it could disconnect some signals from them whenever the selection changed. However, when a scene is closed, its nodes are deleted before the selection change can complete, causing invalid access to dangling pointers.To fix it, the editor now explicitly tells the scene tree dock to clean after itself when the currently edited scene is closing. Rationale for implementation decisions:
SceneTreeDock(as opposed to using a signal) becauseEditorNodealready calls many methods directly on the dock.EditorNode::_remove_edited_scene()since it seems to be the most specific to the situation at hand. Simply closing any scene does not cause the problem, since tracked nodes come from the selection, which applies only to the currently edited scene.