Skip to content

Commit

Permalink
Fix Workspace references being leaked (#17497)
Browse files Browse the repository at this point in the history
We noticed that the `Workspace` was never released (along with the
`Project` and everything that comes along with that) when closing a
window.

After playing around with the LeakDetector and debugging with
`cx.on_release()` callbacks, we found two culprits: the inline assistant
and the outline panel.

Both held strong references to `View<Workspace>` after PR #16589 and PR
#16845.

This PR changes both references to `WeakView<Workspace>` which fixes the
leak but keeps the behaviour the same.

Release Notes:

- N/A

Co-authored-by: Bennet <[email protected]>
  • Loading branch information
mrnugget and bennetbo authored Sep 6, 2024
1 parent e6c1c51 commit 54dd408
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 24 deletions.
5 changes: 4 additions & 1 deletion crates/assistant/src/inline_assistant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,11 @@ impl InlineAssistant {
})
.detach();

let workspace = workspace.clone();
let workspace = workspace.downgrade();
cx.observe_global::<SettingsStore>(move |cx| {
let Some(workspace) = workspace.upgrade() else {
return;
};
let Some(terminal_panel) = workspace.read(cx).panel::<TerminalPanel>(cx) else {
return;
};
Expand Down
57 changes: 34 additions & 23 deletions crates/outline_panel/src/outline_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub struct OutlinePanel {
fs: Arc<dyn Fs>,
width: Option<Pixels>,
project: Model<Project>,
workspace: View<Workspace>,
workspace: WeakView<Workspace>,
active: bool,
pinned: bool,
scroll_handle: UniformListScrollHandle,
Expand Down Expand Up @@ -607,7 +607,7 @@ impl OutlinePanel {

fn new(workspace: &mut Workspace, cx: &mut ViewContext<Workspace>) -> View<Self> {
let project = workspace.project().clone();
let workspace_handle = cx.view().clone();
let workspace_handle = cx.view().downgrade();
let outline_panel = cx.new_view(|cx| {
let filter_editor = cx.new_view(|cx| {
let mut editor = Editor::single_line(cx);
Expand Down Expand Up @@ -865,29 +865,32 @@ impl OutlinePanel {
};

if let Some((offset, anchor)) = scroll_target {
self.workspace
let activate = self
.workspace
.update(cx, |workspace, cx| match self.active_item() {
Some(active_item) => {
workspace.activate_item(active_item.as_ref(), true, change_selection, cx)
}
None => workspace.activate_item(&active_editor, true, change_selection, cx),
});

self.select_entry(entry.clone(), true, cx);
if change_selection {
active_editor.update(cx, |editor, cx| {
editor.change_selections(
Some(Autoscroll::Strategy(AutoscrollStrategy::Top)),
cx,
|s| s.select_ranges(Some(anchor..anchor)),
);
});
active_editor.focus_handle(cx).focus(cx);
} else {
active_editor.update(cx, |editor, cx| {
editor.set_scroll_anchor(ScrollAnchor { offset, anchor }, cx);
});
self.focus_handle.focus(cx);
if activate.is_ok() {
self.select_entry(entry.clone(), true, cx);
if change_selection {
active_editor.update(cx, |editor, cx| {
editor.change_selections(
Some(Autoscroll::Strategy(AutoscrollStrategy::Top)),
cx,
|s| s.select_ranges(Some(anchor..anchor)),
);
});
active_editor.focus_handle(cx).focus(cx);
} else {
active_editor.update(cx, |editor, cx| {
editor.set_scroll_anchor(ScrollAnchor { offset, anchor }, cx);
});
self.focus_handle.focus(cx);
}
}
}
}
Expand Down Expand Up @@ -3316,7 +3319,11 @@ impl OutlinePanel {
let buffer_search = self
.active_item()
.as_deref()
.and_then(|active_item| self.workspace.read(cx).pane_for(active_item))
.and_then(|active_item| {
self.workspace
.upgrade()
.and_then(|workspace| workspace.read(cx).pane_for(active_item))
})
.and_then(|pane| {
pane.read(cx)
.toolbar()
Expand Down Expand Up @@ -3567,8 +3574,10 @@ impl OutlinePanel {
) {
self.pinned = !self.pinned;
if !self.pinned {
if let Some((active_item, active_editor)) =
workspace_active_editor(self.workspace.read(cx), cx)
if let Some((active_item, active_editor)) = self
.workspace
.upgrade()
.and_then(|workspace| workspace_active_editor(workspace.read(cx), cx))
{
if self.should_replace_active_item(active_item.as_ref()) {
self.replace_active_editor(active_item, active_editor, cx);
Expand Down Expand Up @@ -3833,8 +3842,10 @@ impl Panel for OutlinePanel {
let old_active = outline_panel.active;
outline_panel.active = active;
if active && old_active != active {
if let Some((active_item, active_editor)) =
workspace_active_editor(outline_panel.workspace.read(cx), cx)
if let Some((active_item, active_editor)) = outline_panel
.workspace
.upgrade()
.and_then(|workspace| workspace_active_editor(workspace.read(cx), cx))
{
if outline_panel.should_replace_active_item(active_item.as_ref()) {
outline_panel.replace_active_editor(active_item, active_editor, cx);
Expand Down

0 comments on commit 54dd408

Please sign in to comment.