Skip to content

Commit

Permalink
Fix Project strong reference leaks (#22470)
Browse files Browse the repository at this point in the history
Closes #21906

* After #21238,
`TerminalPanel` and `Project` strong references were moved into
`Pane`-related closures, creating a cycle, that did not allow
registering project release and shutting down corresponding language
servers

* After #22329, a special
`Editor` was created with a strong reference to the `Project` which
seemed to do nothing bad in general, but when a working Zed was running
a Zed Dev build, had the same issue with preventing language servers
from shutting down.

The latter is very odd, and seems quite dangerous, as any arbitrary
`Editor` with `Project` in it may do the same, yet it seems that we did
not store them before the way git panel does.

I have tried creating a test, yet seems that we need to initialize a lot
of Zed for it which I failed — all my attempts resulted in a single
language server being present in the `Project`'s statuses.

Release Notes:

- Fixed language servers not being released between project reopens
  • Loading branch information
SomeoneToIgnore committed Dec 28, 2024
1 parent 36018ff commit 600923b
Showing 1 changed file with 23 additions and 11 deletions.
34 changes: 23 additions & 11 deletions crates/terminal_view/src/terminal_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -964,19 +964,23 @@ pub fn new_terminal_pane(
pane.set_should_display_tab_bar(|_| true);
pane.set_zoom_out_on_close(false);

let terminal_panel_for_split_check = terminal_panel.clone();
let split_closure_terminal_panel = terminal_panel.downgrade();
pane.set_can_split(Some(Arc::new(move |pane, dragged_item, cx| {
if let Some(tab) = dragged_item.downcast_ref::<DraggedTab>() {
let current_pane = cx.view().clone();
let can_drag_away =
terminal_panel_for_split_check.update(cx, |terminal_panel, _| {
let is_current_pane = &tab.pane == cx.view();
let Some(can_drag_away) = split_closure_terminal_panel
.update(cx, |terminal_panel, _| {
let current_panes = terminal_panel.center.panes();
!current_panes.contains(&&tab.pane)
|| current_panes.len() > 1
|| (tab.pane != current_pane || pane.items_len() > 1)
});
|| (!is_current_pane || pane.items_len() > 1)
})
.ok()
else {
return false;
};
if can_drag_away {
let item = if tab.pane == current_pane {
let item = if is_current_pane {
pane.item_for_index(tab.ix)
} else {
tab.pane.read(cx).item_for_index(tab.ix)
Expand All @@ -996,7 +1000,12 @@ pub fn new_terminal_pane(
toolbar.add_item(breadcrumbs, cx);
});

let drop_closure_project = project.downgrade();
let drop_closure_terminal_panel = terminal_panel.downgrade();
pane.set_custom_drop_handle(cx, move |pane, dropped_item, cx| {
let Some(project) = drop_closure_project.upgrade() else {
return ControlFlow::Break(());
};
if let Some(tab) = dropped_item.downcast_ref::<DraggedTab>() {
let this_pane = cx.view().clone();
let item = if tab.pane == this_pane {
Expand All @@ -1009,10 +1018,10 @@ pub fn new_terminal_pane(
let source = tab.pane.clone();
let item_id_to_move = item.item_id();

let new_split_pane = pane
let Ok(new_split_pane) = pane
.drag_split_direction()
.map(|split_direction| {
terminal_panel.update(cx, |terminal_panel, cx| {
drop_closure_terminal_panel.update(cx, |terminal_panel, cx| {
let is_zoomed = if terminal_panel.active_pane == this_pane {
pane.is_zoomed()
} else {
Expand All @@ -1033,9 +1042,12 @@ pub fn new_terminal_pane(
anyhow::Ok(new_pane)
})
})
.transpose();
.transpose()
else {
return ControlFlow::Break(());
};

match new_split_pane {
match new_split_pane.transpose() {
// Source pane may be the one currently updated, so defer the move.
Ok(Some(new_pane)) => cx
.spawn(|_, mut cx| async move {
Expand Down

0 comments on commit 600923b

Please sign in to comment.