Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macOS: Drop input handler to avoid editor/project not being dropped #18898

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

mrnugget
Copy link
Member

@mrnugget mrnugget commented Oct 9, 2024

This fixes the problem of a Project sometimes not being dropped when closing the single, last window of Zed.

Turns out, it wasn't get dropped for the following reason:

  1. editor::Editor held a reference to project
  2. The macOS input_handler on the Window held a reference to that Editor
  3. The AppKit window (and its input handler) get dropped asynchronously (in the code in this diff), after the window is closed.
  4. After the window is closed and no cx.update() calls are made anymore, flush_effects is not called anymore.
  5. But flush_effects is where we dropped entities that don't have any more references.

In short: we dropped Editor, which held a reference to Project, out of band, flush_effects wasn't called anymore, and thus the Project wasn't dropped.

cc @ConradIrwin @bennetbo since we talked about this.

Release Notes:

  • N/A

This fixes the problem of a `Project` sometimes not being dropped when
closing the single, last window of Zed.

Turns out, it wasn't get dropped for the following reason:

1. `editor::Editor` held a reference to project
2. The macOS `input_handler` on the `Window` held a reference to that
   `Editor`
3. The AppKit window (and its input handler) get dropped asynchronously
   (in the code in this diff), after the window is closed.
4. After the window is closed and no `cx.update()` calls are made
   anymore, `flush_effects` is not called anymore.
5. But `flush_effects` is where we dropped entities that don't have any
   more references.

In short: we dropped `Editor`, which held a reference to `Project`, out
of band, `flush_effects` wasn't called anymore, and thus the `Project`
wasn't dropped.

Co-authored-by: Antonio <antonio@zed.dev>
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Oct 9, 2024
@zed-industries-bot
Copy link

Warnings
⚠️
macOS: Drop input handler to avoid editor/project not being dropped
^

Write PR titles using sentence case.

Have feedback on this plugin? Let's hear it!

Generated by 🚫 dangerJS against 4622915

@mrnugget mrnugget merged commit 9c54bd1 into main Oct 9, 2024
9 checks passed
@mrnugget mrnugget deleted the fix-project-not-dropped branch October 9, 2024 08:45
noaccOS pushed a commit to noaccOS/zed that referenced this pull request Oct 19, 2024
…ed-industries#18898)

This fixes the problem of a `Project` sometimes not being dropped when
closing the single, last window of Zed.

Turns out, it wasn't get dropped for the following reason:

1. `editor::Editor` held a reference to project
2. The macOS `input_handler` on the `Window` held a reference to that
`Editor`
3. The AppKit window (and its input handler) get dropped asynchronously
(in the code in this diff), after the window is closed.
4. After the window is closed and no `cx.update()` calls are made
anymore, `flush_effects` is not called anymore.
5. But `flush_effects` is where we dropped entities that don't have any
more references.

In short: we dropped `Editor`, which held a reference to `Project`, out
of band, `flush_effects` wasn't called anymore, and thus the `Project`
wasn't dropped.

cc @ConradIrwin @bennetbo since we talked about this.

Release Notes:

- N/A

Co-authored-by: Antonio <antonio@zed.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants