fix(workspace): invalidate active pane on pane cleanup to prevent invalid widget references
- commit
- 1617d38d30c3e413cfbee42910dcafee73a65103
- author
- Brice · · 2025-10-04 21:08 UTC
- committer
- Brice · · 2025-10-05 13:54 UTC
- parent
- 6611f45
- signature
- unsigned
2 files · +77 -9
M internal/app/browser/focus_state_machine.go +27 -0
| @@ -420,6 +420,33 @@ func (fsm *FocusStateMachine) GetActivePane() *paneNode { | |||
| 420 | 420 | return fsm.activePane | |
| 421 | 421 | } | |
| 422 | 422 | ||
| 423 | + | // InvalidateActivePane clears the active pane if it matches the provided node. | |
| 424 | + | // This is used when panes are destroyed so the focus state machine doesn't keep | |
| 425 | + | // references to invalid widgets. | |
| 426 | + | func (fsm *FocusStateMachine) InvalidateActivePane(node *paneNode) { | |
| 427 | + | if node == nil { | |
| 428 | + | return | |
| 429 | + | } | |
| 430 | + | ||
| 431 | + | fsm.mu.Lock() | |
| 432 | + | defer fsm.mu.Unlock() | |
| 433 | + | ||
| 434 | + | if fsm.activePane != node { | |
| 435 | + | return | |
| 436 | + | } | |
| 437 | + | ||
| 438 | + | log.Printf("[FSM] Active pane invalidated: %p", node) | |
| 439 | + | fsm.activePane = nil | |
| 440 | + | ||
| 441 | + | if fsm.wm != nil && fsm.wm.app != nil { | |
| 442 | + | fsm.wm.app.activePane = nil | |
| 443 | + | } | |
| 444 | + | ||
| 445 | + | if fsm.currentState == StateFocused { | |
| 446 | + | fsm.currentState = StateIdle | |
| 447 | + | } | |
| 448 | + | } | |
| 449 | + | ||
| 423 | 450 | // GetCurrentState returns the current state of the focus system | |
| 424 | 451 | func (fsm *FocusStateMachine) GetCurrentState() FocusState { | |
| 425 | 452 | fsm.mu.RLock() | |
| @@ -420,6 +420,33 @@ func (fsm *FocusStateMachine) GetActivePane() *paneNode { | |||||
| 420 | return fsm.activePane | 420 | return fsm.activePane | ||
| 421 | } | 421 | } | ||
| 422 | 422 | ||||
| 423 | + | // InvalidateActivePane clears the active pane if it matches the provided node. | |||
| 424 | + | // This is used when panes are destroyed so the focus state machine doesn't keep | |||
| 425 | + | // references to invalid widgets. | |||
| 426 | + | func (fsm *FocusStateMachine) InvalidateActivePane(node *paneNode) { | |||
| 427 | + | if node == nil { | |||
| 428 | + | return | |||
| 429 | + | } | |||
| 430 | + | ||||
| 431 | + | fsm.mu.Lock() | |||
| 432 | + | defer fsm.mu.Unlock() | |||
| 433 | + | ||||
| 434 | + | if fsm.activePane != node { | |||
| 435 | + | return | |||
| 436 | + | } | |||
| 437 | + | ||||
| 438 | + | log.Printf("[FSM] Active pane invalidated: %p", node) | |||
| 439 | + | fsm.activePane = nil | |||
| 440 | + | ||||
| 441 | + | if fsm.wm != nil && fsm.wm.app != nil { | |||
| 442 | + | fsm.wm.app.activePane = nil | |||
| 443 | + | } | |||
| 444 | + | ||||
| 445 | + | if fsm.currentState == StateFocused { | |||
| 446 | + | fsm.currentState = StateIdle | |||
| 447 | + | } | |||
| 448 | + | } | |||
| 449 | + | ||||
| 423 | // GetCurrentState returns the current state of the focus system | 450 | // GetCurrentState returns the current state of the focus system | ||
| 424 | func (fsm *FocusStateMachine) GetCurrentState() FocusState { | 451 | func (fsm *FocusStateMachine) GetCurrentState() FocusState { | ||
| 425 | fsm.mu.RLock() | 452 | fsm.mu.RLock() | ||
M internal/app/browser/workspace_pane_ops.go +50 -9
| @@ -190,6 +190,18 @@ func (wm *WorkspaceManager) closeCurrentPane() { | |||
| 190 | 190 | // leftmostLeaf finds the leftmost leaf node in a subtree | |
| 191 | 191 | func (wm *WorkspaceManager) leftmostLeaf(node *paneNode) *paneNode { | |
| 192 | 192 | for node != nil && !node.isLeaf { | |
| 193 | + | if node.isStacked { | |
| 194 | + | if node.activeStackIndex >= 0 && node.activeStackIndex < len(node.stackedPanes) { | |
| 195 | + | candidate := node.stackedPanes[node.activeStackIndex] | |
| 196 | + | if candidate != nil { | |
| 197 | + | return candidate | |
| 198 | + | } | |
| 199 | + | } | |
| 200 | + | if len(node.stackedPanes) > 0 { | |
| 201 | + | return node.stackedPanes[0] | |
| 202 | + | } | |
| 203 | + | return nil | |
| 204 | + | } | |
| 193 | 205 | if node.left != nil { | |
| 194 | 206 | node = node.left | |
| 195 | 207 | continue | |
| @@ -1019,23 +1031,48 @@ func (wm *WorkspaceManager) attachRoot(root *paneNode) { | |||
| 1019 | 1031 | return | |
| 1020 | 1032 | } | |
| 1021 | 1033 | ||
| 1022 | - | // Clear any existing window child to avoid GTK warnings when swapping roots. | |
| 1023 | - | wm.window.SetChild(0) | |
| 1034 | + | // Hold a transient reference so GTK does not destroy the widget while detached | |
| 1035 | + | releaseRef := webkit.WidgetRef(root.container) | |
| 1036 | + | if !releaseRef { | |
| 1037 | + | log.Printf("[workspace] WARNING: failed to acquire temporary ref for widget %#x; aborting attachRoot", root.container) | |
| 1038 | + | return | |
| 1039 | + | } | |
| 1040 | + | defer func() { | |
| 1041 | + | if webkit.WidgetIsValid(root.container) { | |
| 1042 | + | webkit.WidgetUnref(root.container) | |
| 1043 | + | } | |
| 1044 | + | }() | |
| 1024 | 1045 | ||
| 1025 | - | // Unparent from the previous container (paned, stack, etc.) before reattaching. | |
| 1046 | + | // Detach from previous container (paned, stack, etc.) before replacing the window child. | |
| 1026 | 1047 | if parent := webkit.WidgetGetParent(root.container); parent != 0 { | |
| 1027 | 1048 | log.Printf("[workspace] unparenting widget %#x from parent %#x before window attach", root.container, parent) | |
| 1028 | 1049 | webkit.WidgetUnparent(root.container) | |
| 1050 | + | if !webkit.WidgetIsValid(root.container) { | |
| 1051 | + | log.Printf("[workspace] WARNING: widget %#x became invalid during unparent; aborting attachRoot", root.container) | |
| 1052 | + | return | |
| 1053 | + | } | |
| 1029 | 1054 | } | |
| 1030 | 1055 | ||
| 1031 | - | webkit.WidgetSetHExpand(root.container, true) | |
| 1032 | - | webkit.WidgetSetVExpand(root.container, true) | |
| 1056 | + | // Clear any existing window child to avoid GTK warnings when swapping roots. | |
| 1057 | + | wm.window.SetChild(0) | |
| 1058 | + | ||
| 1059 | + | if webkit.WidgetIsValid(root.container) { | |
| 1060 | + | webkit.WidgetSetHExpand(root.container, true) | |
| 1061 | + | webkit.WidgetSetVExpand(root.container, true) | |
| 1062 | + | } | |
| 1063 | + | ||
| 1064 | + | if !webkit.WidgetIsValid(root.container) { | |
| 1065 | + | log.Printf("[workspace] WARNING: widget %#x invalid prior to window attach", root.container) | |
| 1066 | + | return | |
| 1067 | + | } | |
| 1033 | 1068 | ||
| 1034 | 1069 | wm.window.SetChild(root.container) | |
| 1035 | - | webkit.WidgetQueueAllocate(root.container) | |
| 1036 | - | webkit.WidgetShow(root.container) | |
| 1037 | - | webkit.WidgetQueueResize(root.container) | |
| 1038 | - | webkit.WidgetQueueDraw(root.container) | |
| 1070 | + | if webkit.WidgetIsValid(root.container) { | |
| 1071 | + | webkit.WidgetQueueAllocate(root.container) | |
| 1072 | + | webkit.WidgetShow(root.container) | |
| 1073 | + | webkit.WidgetQueueResize(root.container) | |
| 1074 | + | webkit.WidgetQueueDraw(root.container) | |
| 1075 | + | } | |
| 1039 | 1076 | ||
| 1040 | 1077 | wm.scheduleIdleGuarded(func() bool { | |
| 1041 | 1078 | if root == nil || !root.widgetValid { | |
| @@ -1065,6 +1102,10 @@ func (wm *WorkspaceManager) cleanupPane(node *paneNode, generation uint) { | |||
| 1065 | 1102 | return | |
| 1066 | 1103 | } | |
| 1067 | 1104 | ||
| 1105 | + | if wm.focusStateMachine != nil { | |
| 1106 | + | wm.focusStateMachine.InvalidateActivePane(node) | |
| 1107 | + | } | |
| 1108 | + | ||
| 1068 | 1109 | wm.cancelIdleHandles(node) | |
| 1069 | 1110 | ||
| 1070 | 1111 | node.widgetValid = false | |
| @@ -190,6 +190,18 @@ func (wm *WorkspaceManager) closeCurrentPane() { | |||||
| 190 | // leftmostLeaf finds the leftmost leaf node in a subtree | 190 | // leftmostLeaf finds the leftmost leaf node in a subtree | ||
| 191 | func (wm *WorkspaceManager) leftmostLeaf(node *paneNode) *paneNode { | 191 | func (wm *WorkspaceManager) leftmostLeaf(node *paneNode) *paneNode { | ||
| 192 | for node != nil && !node.isLeaf { | 192 | for node != nil && !node.isLeaf { | ||
| 193 | + | if node.isStacked { | |||
| 194 | + | if node.activeStackIndex >= 0 && node.activeStackIndex < len(node.stackedPanes) { | |||
| 195 | + | candidate := node.stackedPanes[node.activeStackIndex] | |||
| 196 | + | if candidate != nil { | |||
| 197 | + | return candidate | |||
| 198 | + | } | |||
| 199 | + | } | |||
| 200 | + | if len(node.stackedPanes) > 0 { | |||
| 201 | + | return node.stackedPanes[0] | |||
| 202 | + | } | |||
| 203 | + | return nil | |||
| 204 | + | } | |||
| 193 | if node.left != nil { | 205 | if node.left != nil { | ||
| 194 | node = node.left | 206 | node = node.left | ||
| 195 | continue | 207 | continue | ||
| @@ -1019,23 +1031,48 @@ func (wm *WorkspaceManager) attachRoot(root *paneNode) { | |||||
| 1019 | return | 1031 | return | ||
| 1020 | } | 1032 | } | ||
| 1021 | 1033 | ||||
| 1022 | - | // Clear any existing window child to avoid GTK warnings when swapping roots. | 1034 | + | // Hold a transient reference so GTK does not destroy the widget while detached |
| 1023 | - | wm.window.SetChild(0) | 1035 | + | releaseRef := webkit.WidgetRef(root.container) |
| 1036 | + | if !releaseRef { | |||
| 1037 | + | log.Printf("[workspace] WARNING: failed to acquire temporary ref for widget %#x; aborting attachRoot", root.container) | |||
| 1038 | + | return | |||
| 1039 | + | } | |||
| 1040 | + | defer func() { | |||
| 1041 | + | if webkit.WidgetIsValid(root.container) { | |||
| 1042 | + | webkit.WidgetUnref(root.container) | |||
| 1043 | + | } | |||
| 1044 | + | }() | |||
| 1024 | 1045 | ||||
| 1025 | - | // Unparent from the previous container (paned, stack, etc.) before reattaching. | 1046 | + | // Detach from previous container (paned, stack, etc.) before replacing the window child. |
| 1026 | if parent := webkit.WidgetGetParent(root.container); parent != 0 { | 1047 | if parent := webkit.WidgetGetParent(root.container); parent != 0 { | ||
| 1027 | log.Printf("[workspace] unparenting widget %#x from parent %#x before window attach", root.container, parent) | 1048 | log.Printf("[workspace] unparenting widget %#x from parent %#x before window attach", root.container, parent) | ||
| 1028 | webkit.WidgetUnparent(root.container) | 1049 | webkit.WidgetUnparent(root.container) | ||
| 1050 | + | if !webkit.WidgetIsValid(root.container) { | |||
| 1051 | + | log.Printf("[workspace] WARNING: widget %#x became invalid during unparent; aborting attachRoot", root.container) | |||
| 1052 | + | return | |||
| 1053 | + | } | |||
| 1029 | } | 1054 | } | ||
| 1030 | 1055 | ||||
| 1031 | - | webkit.WidgetSetHExpand(root.container, true) | 1056 | + | // Clear any existing window child to avoid GTK warnings when swapping roots. |
| 1032 | - | webkit.WidgetSetVExpand(root.container, true) | 1057 | + | wm.window.SetChild(0) |
| 1058 | + | ||||
| 1059 | + | if webkit.WidgetIsValid(root.container) { | |||
| 1060 | + | webkit.WidgetSetHExpand(root.container, true) | |||
| 1061 | + | webkit.WidgetSetVExpand(root.container, true) | |||
| 1062 | + | } | |||
| 1063 | + | ||||
| 1064 | + | if !webkit.WidgetIsValid(root.container) { | |||
| 1065 | + | log.Printf("[workspace] WARNING: widget %#x invalid prior to window attach", root.container) | |||
| 1066 | + | return | |||
| 1067 | + | } | |||
| 1033 | 1068 | ||||
| 1034 | wm.window.SetChild(root.container) | 1069 | wm.window.SetChild(root.container) | ||
| 1035 | - | webkit.WidgetQueueAllocate(root.container) | 1070 | + | if webkit.WidgetIsValid(root.container) { |
| 1036 | - | webkit.WidgetShow(root.container) | 1071 | + | webkit.WidgetQueueAllocate(root.container) |
| 1037 | - | webkit.WidgetQueueResize(root.container) | 1072 | + | webkit.WidgetShow(root.container) |
| 1038 | - | webkit.WidgetQueueDraw(root.container) | 1073 | + | webkit.WidgetQueueResize(root.container) |
| 1074 | + | webkit.WidgetQueueDraw(root.container) | |||
| 1075 | + | } | |||
| 1039 | 1076 | ||||
| 1040 | wm.scheduleIdleGuarded(func() bool { | 1077 | wm.scheduleIdleGuarded(func() bool { | ||
| 1041 | if root == nil || !root.widgetValid { | 1078 | if root == nil || !root.widgetValid { | ||
| @@ -1065,6 +1102,10 @@ func (wm *WorkspaceManager) cleanupPane(node *paneNode, generation uint) { | |||||
| 1065 | return | 1102 | return | ||
| 1066 | } | 1103 | } | ||
| 1067 | 1104 | ||||
| 1105 | + | if wm.focusStateMachine != nil { | |||
| 1106 | + | wm.focusStateMachine.InvalidateActivePane(node) | |||
| 1107 | + | } | |||
| 1108 | + | ||||
| 1068 | wm.cancelIdleHandles(node) | 1109 | wm.cancelIdleHandles(node) | ||
| 1069 | 1110 | ||||
| 1070 | node.widgetValid = false | 1111 | node.widgetValid = false | ||