Skip to content

gpui: Remove unsound await_on_background helper - #56132

Merged
Veykril merged 1 commit into
mainfrom
push-xswqmsotmmyx
May 8, 2026
Merged

gpui: Remove unsound await_on_background helper#56132
Veykril merged 1 commit into
mainfrom
push-xswqmsotmmyx

Conversation

@Veykril

@Veykril Veykril commented May 8, 2026

Copy link
Copy Markdown
Member

The function is unsound due to the classic fact that one can leak tasks, sidestepping the blocking drop behavior resulting in a use after free.

Release Notes:

  • N/A or Added/Fixed/Improved ...

@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label May 8, 2026
@zed-community-bot zed-community-bot Bot added the staff Pull requests authored by a current member of Zed staff label May 8, 2026
@Veykril
Veykril force-pushed the push-xswqmsotmmyx branch 4 times, most recently from 08a8371 to 2f0bfbd Compare May 8, 2026 06:50

@Veykril Veykril left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The language crate changes are merely switching out the async model to be impl Future where the Future is effectively 'static so we can offload it to the background.

@Veykril Veykril changed the title gpui_windows: Avoid process-wide priority elevation gpui: Remove unsound await_on_background helper May 8, 2026
@Veykril
Veykril force-pushed the push-xswqmsotmmyx branch from 2f0bfbd to 7e295c1 Compare May 8, 2026 06:53

@SomeoneToIgnore SomeoneToIgnore left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to

  • get some elaboration on unsoundness in the PR description as it's always interesting
  • have a way to avoid repeating impl Future<Output = T> + use<> and use some shorter form

but nothing blocking.

)
.await;
delegate: &Arc<dyn LspAdapterDelegate>,
) -> impl Send + Future<Output = Option<lsp::LanguageServerBinary>> + use<> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if some generic type alias is ok to have for this?
Maybe not in gpui or other public place, but some utils/runtime crate?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would need to be a trait alias which is not a stable thing yet

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or impl Foo for Send + ... and require that instead?
Either way, not too big of a deal.

Comment thread crates/language/src/language.rs Outdated
@Veykril
Veykril force-pushed the push-xswqmsotmmyx branch from 7e295c1 to 27e48ff Compare May 8, 2026 09:09
@Veykril
Veykril enabled auto-merge May 8, 2026 09:09
@Veykril
Veykril added this pull request to the merge queue May 8, 2026
Merged via the queue into main with commit c8f0026 May 8, 2026
31 checks passed
@Veykril
Veykril deleted the push-xswqmsotmmyx branch May 8, 2026 09:33
@kylekz kylekz mentioned this pull request May 8, 2026
3 tasks
jonx pushed a commit to jonx/zed-aros that referenced this pull request Jul 17, 2026
The function is unsound due to the classic fact that one can leak tasks,
sidestepping the blocking drop behavior resulting in a use after free.

Release Notes:

- N/A or Added/Fixed/Improved ...
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
The function is unsound due to the classic fact that one can leak tasks,
sidestepping the blocking drop behavior resulting in a use after free.

Release Notes:

- N/A or Added/Fixed/Improved ...
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 staff Pull requests authored by a current member of Zed staff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants