gpui: Fix send_file_drop_event holding mutex during event callback#50173
Merged
MrSubidubi merged 1 commit intozed-industries:mainfrom Feb 26, 2026
Merged
Conversation
`send_file_drop_event` was the only event dispatch site in window.rs that called the event callback while still holding the `MacWindowState` mutex. every other callback site in the file uses the established `take() / drop(lock) / callback()` pattern, which releases the lock before invoking user code. this is a latent code quality issue: no deadlock occurs today because the current file-drop callback chain does not re-enter `MacWindowState`'s mutex (`Window::mouse_position()` returns a cached field, `Window::refresh()` only marks a dirty flag, and `cx.activate()` uses a separate `MacPlatform` mutex). however, `parking_lot::Mutex` is not reentrant, so any future platform call added inside a file-drop handler would deadlock immediately and silently. the fix brings `send_file_drop_event` in line with the rest of the file by taking the callback out of the state, dropping the lock, invoking the callback, then re-acquiring the lock to restore the callback and update state.
cla-bot
bot
added
the
cla-signed
Contributor
Author
|
@MrSubidubi hey, can you check this out? only if u have some free time |
maxdeviant
changed the title
MrSubidubi
changed the title
send_file_drop_event holding mutex during event callback
MrSubidubi
approved these changes
Feb 26, 2026
Member
MrSubidubi
left a comment
MrSubidubi
left a comment
There was a problem hiding this comment.
Nice catch and follow-up, thank you!
Contributor
Author
|
@MrSubidubi i was honestly worried that this wont be worth reviewing or merging |
Member
|
Stuff like this is totally fine and you also add good explanations, which helps with reviewing. No worries! |
Contributor
Author
|
@MrSubidubi thats really good to hear, because i pinned about a dozen more of these but didnt have to confidence to push them. also if you dont mind me asking are you in charge of reviewing everything macos related? |
Member
Nope, not at all. You are free to push them (maybe not all at one, though 😅 ) and we'll eventually take a look. It can take some time (and no need to ping, we'll see what's coming in), sometimes longer than we'd like to, but eventually we will get back to you 🙂 |
Contributor
Author
|
@MrSubidubi ok. im genuinely sorry for the bother tho. I'll make sure to let u guys do your thing. the thing is i need all the oss merges i can get to pass 10th grade lol |
Member
|
You are free to send them, as mentioned above, given that they also aligh with https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md. So no need to hesistate, probably just don't send 5 PRs at one 😅 |
rtfeldman
pushed a commit
that referenced
this pull request
Feb 27, 2026
…50173) `send_file_drop_event` was the only event dispatch site in window.rs that called the event callback while still holding the `MacWindowState` mutex. every other callback site in the file uses the established `take() / drop(lock) / callback()` pattern which releases the lock before invoking user code. this is a latent code quality issue: no deadlock occurs today because the current file-drop callback chain does not reenter `MacWindowState`'s mutex (`Window::mouse_position()` returns a cached field `Window::refresh()` only marks a dirty flag and `cx.activate()` uses a separate `MacPlatform` mutex). but `parking_lot::Mutex` is not entering so any future platform call added inside a filedrop handler would deadlock immediately and silently. the fix brings `send_file_drop_event` in line with the rest of the file by taking the callback out of the state dropping the lock invoking the callback then reacquiring the lock to restore the callback and update state. Release Notes: - N/A
tahayvr
pushed a commit
to tahayvr/zed
that referenced
this pull request
Mar 4, 2026
…ed-industries#50173) `send_file_drop_event` was the only event dispatch site in window.rs that called the event callback while still holding the `MacWindowState` mutex. every other callback site in the file uses the established `take() / drop(lock) / callback()` pattern which releases the lock before invoking user code. this is a latent code quality issue: no deadlock occurs today because the current file-drop callback chain does not reenter `MacWindowState`'s mutex (`Window::mouse_position()` returns a cached field `Window::refresh()` only marks a dirty flag and `cx.activate()` uses a separate `MacPlatform` mutex). but `parking_lot::Mutex` is not entering so any future platform call added inside a filedrop handler would deadlock immediately and silently. the fix brings `send_file_drop_event` in line with the rest of the file by taking the callback out of the state dropping the lock invoking the callback then reacquiring the lock to restore the callback and update state. Release Notes: - N/A
naaiyy
added a commit
to Glass-HQ/gpui
that referenced
this pull request
Mar 10, 2026
Port of upstream zed-industries/zed#50173. Releases the MacWindowState lock before invoking the event callback, matching the take/drop/callback pattern used by every other dispatch site in window.rs.
naaiyy
added a commit
to Glass-HQ/Glass
that referenced
this pull request
Mar 10, 2026
…ed-industries#50173) Upstream commit dd9efd9 applied to the separate GPUI repository. No changes needed in Glass itself.
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
2 participants
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.
send_file_drop_eventwas the only event dispatch site in window.rs that called the event callback while still holding theMacWindowStatemutex. every other callback site in the file uses the establishedtake() / drop(lock) / callback()pattern which releases the lock before invoking user code.this is a latent code quality issue: no deadlock occurs today because the current file-drop callback chain does not reenter
MacWindowState's mutex (Window::mouse_position()returns a cached fieldWindow::refresh()only marks a dirty flag andcx.activate()uses a separateMacPlatformmutex). butparking_lot::Mutexis not entering so any future platform call added inside a filedrop handler would deadlock immediately and silently.the fix brings
send_file_drop_eventin line with the rest of the file by taking the callback out of the state dropping the lock invoking the callback then reacquiring the lock to restore the callback and update state.Release Notes: