Fix race condition hiding tabs moved between windows and mutex leak#175
Open
duncanplatt wants to merge 1 commit intophotodiode:masterfrom
Open
Fix race condition hiding tabs moved between windows and mutex leak#175duncanplatt wants to merge 1 commit intophotodiode:masterfrom
duncanplatt wants to merge 1 commit intophotodiode:masterfrom
Conversation
Fix race condition hiding tabs moved between windows and mutex leak Firefox 148 landed sync about:blank (Bug 543435), which changed tab adoption timing and introduced a race between the onAttached and onActivated event handlers. When a tab is dragged between windows, activated() could call toggleVisibleTabs before attached() finished updating the tab's groupId, causing the moved tab to be hidden. Three issues fixed in addon.tabs.events.js: 1. activated() now persists the corrected groupId when it detects the active tab's group doesn't exist in the current window. Previously the fallback was only used locally for that one toggleVisibleTabs call, so the tab would be re-hidden on the next activation. 2. attached() now explicitly shows the tab after reassigning its group, recovering visibility if a racing toggleVisibleTabs already hid it. Also fixed in addon.tabGroups.js: 3. setActiveId() returned early without releasing the mutex when the group was not found, which would deadlock all subsequent group operations (create, query, update, remove). Refs: Firefox Bug 543435, Bug 1987344, Bug 2002643 Refs: photodiode#173
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
1 participant
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.
Fix race condition hiding tabs moved between windows and mutex leak
Firefox 148 landed sync about:blank (Bug 543435), which changed tab adoption timing and introduced a race between the onAttached and onActivated event handlers. When a tab is dragged between windows, activated() could call toggleVisibleTabs before attached() finished updating the tab's groupId, causing the moved tab to be hidden.
Three issues fixed in addon.tabs.events.js:
activated() now persists the corrected groupId when it detects the active tab's group doesn't exist in the current window. Previously the fallback was only used locally for that one toggleVisibleTabs call, so the tab would be re-hidden on the next activation.
attached() now explicitly shows the tab after reassigning its group, recovering visibility if a racing toggleVisibleTabs already hid it.
Also fixed in addon.tabGroups.js:
Refs: Firefox Bug 543435, Bug 1987344, Bug 2002643
Refs: #173