chore: Fix subscriptions leak. (#597)

crates/story/src/title_bar.rs

@@ -3,7 +3,7 @@ use std::rc::Rc;
 use gpui::{
     div, px, AnyElement, App, AppContext, ClickEvent, Context, Corner, Entity, FocusHandle, Hsla,
     InteractiveElement as _, IntoElement, MouseButton, ParentElement as _, Render, SharedString,
-    Styled as _, Window,
+    Styled as _, Subscription, Window,
 };
 use ui::{
     badge::Badge,
@@ -24,6 +24,7 @@ pub struct AppTitleBar {
     font_size_selector: Entity<FontSizeSelector>,
     theme_color_picker: Entity<ColorPicker>,
     child: Rc<dyn Fn(&mut Window, &mut App) -> AnyElement>,
+    _subscriptions: Vec<Subscription>,
 }
 
 impl AppTitleBar {
@@ -43,7 +44,8 @@ impl AppTitleBar {
             picker.set_value(cx.theme().primary, window, cx);
             picker
         });
-        cx.subscribe_in(
+
+        let _subscriptions = vec![cx.subscribe_in(
             &theme_color_picker,
             window,
             |this, _, ev: &ColorPickerEvent, window, cx| match ev {
@@ -51,8 +53,7 @@ impl AppTitleBar {
                     this.set_theme_color(*color, window, cx);
                 }
             },
-        )
+        )];
-        .detach();
 
         Self {
             title: title.into(),
@@ -61,6 +62,7 @@ impl AppTitleBar {
             font_size_selector,
             theme_color_picker,
             child: Rc::new(|_, _| div().into_any_element()),
+            _subscriptions,
         }
     }
 

crates/ui/src/color_picker.rs

@@ -2,7 +2,7 @@ use gpui::{
     anchored, canvas, deferred, div, prelude::FluentBuilder as _, px, relative, App, AppContext,
     Bounds, Context, Corner, ElementId, Entity, EventEmitter, FocusHandle, Focusable, Hsla,
     InteractiveElement as _, IntoElement, KeyBinding, MouseButton, ParentElement, Pixels, Point,
-    Render, SharedString, StatefulInteractiveElement as _, Styled, Window,
+    Render, SharedString, StatefulInteractiveElement as _, Styled, Subscription, Window,
 };
 
 use crate::{
@@ -66,13 +66,14 @@ pub struct ColorPicker {
 
     open: bool,
     bounds: Bounds<Pixels>,
+    _subscriptions: Vec<Subscription>,
 }
 
 impl ColorPicker {
     pub fn new(id: impl Into<ElementId>, window: &mut Window, cx: &mut Context<Self>) -> Self {
         let color_input = cx.new(|cx| TextInput::new(window, cx).xsmall());
 
-        cx.subscribe_in(
+        let _subscriptions = vec![cx.subscribe_in(
             &color_input,
             window,
             |this, _, ev: &InputEvent, window, cx| match ev {
@@ -91,8 +92,7 @@ impl ColorPicker {
                 }
                 _ => {}
             },
-        )
+        )];
-        .detach();
 
         Self {
             id: id.into(),
@@ -118,6 +118,7 @@ impl ColorPicker {
             color_input,
             open: false,
             bounds: Bounds::default(),
+            _subscriptions,
         }
     }
 

crates/ui/src/dropdown.rs

@@ -2,7 +2,8 @@ use gpui::{
     actions, anchored, canvas, deferred, div, prelude::FluentBuilder, px, rems, AnyElement, App,
     AppContext, Bounds, ClickEvent, Context, DismissEvent, ElementId, Entity, EventEmitter,
     FocusHandle, Focusable, InteractiveElement, IntoElement, KeyBinding, Length, ParentElement,
-    Pixels, Render, SharedString, StatefulInteractiveElement, Styled, Task, WeakEntity, Window,
+    Pixels, Render, SharedString, StatefulInteractiveElement, Styled, Subscription, Task,
+    WeakEntity, Window,
 };
 use rust_i18n::t;
 
@@ -252,6 +253,7 @@ pub struct Dropdown<D: DropdownDelegate + 'static> {
     /// Store the bounds of the input
     bounds: Bounds<Pixels>,
     disabled: bool,
+    _subscriptions: Vec<Subscription>,
 }
 
 pub struct SearchableVec<T> {
@@ -352,9 +354,10 @@ where
             list
         });
 
-        cx.on_blur(&list.focus_handle(cx), window, Self::on_blur)
+        let _subscriptions = vec![
-            .detach();
+            cx.on_blur(&list.focus_handle(cx), window, Self::on_blur),
-        cx.on_blur(&focus_handle, window, Self::on_blur).detach();
+            cx.on_blur(&focus_handle, window, Self::on_blur),
+        ];
 
         let mut this = Self {
             id: id.into(),
@@ -372,6 +375,7 @@ where
             menu_width: Length::Auto,
             bounds: Bounds::default(),
             disabled: false,
+            _subscriptions,
         };
         this.set_selected_index(selected_index, window, cx);
         this

crates/ui/src/input/input.rs

@@ -15,7 +15,7 @@ use gpui::{
     Context, Entity, EntityInputHandler, EventEmitter, FocusHandle, Focusable,
     InteractiveElement as _, IntoElement, KeyBinding, KeyDownEvent, MouseButton, MouseDownEvent,
     MouseMoveEvent, MouseUpEvent, ParentElement as _, Pixels, Point, Rems, Render, ScrollHandle,
-    ScrollWheelEvent, SharedString, Styled as _, UTF16Selection, Window, WrappedLine,
+    ScrollWheelEvent, SharedString, Styled as _, Subscription, UTF16Selection, Window, WrappedLine,
 };
 
 // TODO:
@@ -230,6 +230,7 @@ pub struct TextInput {
     pub(crate) scroll_size: gpui::Size<Pixels>,
     /// To remember the horizontal column (x-coordinate) of the cursor position.
     preferred_x_offset: Option<Pixels>,
+    _subscriptions: Vec<Subscription>,
 }
 
 impl EventEmitter<InputEvent> for TextInput {}
@@ -239,7 +240,26 @@ impl TextInput {
         let focus_handle = cx.focus_handle();
         let blink_cursor = cx.new(|_| BlinkCursor::new());
         let history = History::new().group_interval(std::time::Duration::from_secs(1));
-        let input = Self {
+
+        let _subscriptions = vec![
+            // Observe the blink cursor to repaint the view when it changes.
+            cx.observe(&blink_cursor, |_, _, cx| cx.notify()),
+            // Blink the cursor when the window is active, pause when it's not.
+            cx.observe_window_activation(window, |input, window, cx| {
+                if window.is_window_active() {
+                    let focus_handle = input.focus_handle.clone();
+                    if focus_handle.is_focused(window) {
+                        input.blink_cursor.update(cx, |blink_cursor, cx| {
+                            blink_cursor.start(cx);
+                        });
+                    }
+                }
+            }),
+            cx.on_focus(&focus_handle, window, Self::on_focus),
+            cx.on_blur(&focus_handle, window, Self::on_blur),
+        ];
+
+        Self {
             focus_handle: focus_handle.clone(),
             text: "".into(),
             multi_line: false,
@@ -272,29 +292,9 @@ impl TextInput {
             scrollbar_state: Rc::new(Cell::new(ScrollbarState::default())),
             scroll_size: gpui::size(px(0.), px(0.)),
             preferred_x_offset: None,
-        };
+            _subscriptions,
-
-        // Observe the blink cursor to repaint the view when it changes.
-        cx.observe(&input.blink_cursor, |_, _, cx| cx.notify())
-            .detach();
-        // Blink the cursor when the window is active, pause when it's not.
-        cx.observe_window_activation(window, |input, window, cx| {
-            if window.is_window_active() {
-                let focus_handle = input.focus_handle.clone();
-                if focus_handle.is_focused(window) {
-                    input.blink_cursor.update(cx, |blink_cursor, cx| {
-                        blink_cursor.start(cx);
-                    });
         }
     }
-        })
-        .detach();
-
-        cx.on_focus(&focus_handle, window, Self::on_focus).detach();
-        cx.on_blur(&focus_handle, window, Self::on_blur).detach();
-
-        input
-    }
 
     /// Use the text input field as a multi-line Textarea.
     pub fn multi_line(mut self) -> Self {

crates/ui/src/input/otp_input.rs

@@ -1,7 +1,7 @@
 use gpui::{
     div, prelude::FluentBuilder, px, AnyElement, AppContext as _, Context, Entity, EventEmitter,
     FocusHandle, Focusable, InteractiveElement, IntoElement, KeyDownEvent, MouseButton,
-    MouseDownEvent, ParentElement as _, Render, SharedString, Styled as _, Window,
+    MouseDownEvent, ParentElement as _, Render, SharedString, Styled as _, Subscription, Window,
 };
 
 use crate::{h_flex, v_flex, ActiveTheme, Icon, IconName, Sizable, Size};
@@ -29,24 +29,17 @@ pub struct OtpInput {
     value: SharedString,
     blink_cursor: Entity<BlinkCursor>,
     size: Size,
+    _subscriptions: Vec<Subscription>,
 }
 
 impl OtpInput {
     pub fn new(length: usize, window: &mut Window, cx: &mut Context<Self>) -> Self {
         let focus_handle = cx.focus_handle();
         let blink_cursor = cx.new(|_| BlinkCursor::new());
-        let input = Self {
-            focus_handle: focus_handle.clone(),
-            length,
-            number_of_groups: 2,
-            value: SharedString::default(),
-            masked: false,
-            blink_cursor: blink_cursor.clone(),
-            size: Size::Medium,
-        };
 
+        let _subscriptions = vec![
             // Observe the blink cursor to repaint the view when it changes.
-        cx.observe(&blink_cursor, |_, _, cx| cx.notify()).detach();
+            cx.observe(&blink_cursor, |_, _, cx| cx.notify()),
             // Blink the cursor when the window is active, pause when it's not.
             cx.observe_window_activation(window, |this, window, cx| {
                 if window.is_window_active() {
@@ -57,13 +50,21 @@ impl OtpInput {
                         });
                     }
                 }
-        })
+            }),
-        .detach();
+            cx.on_focus(&focus_handle, window, Self::on_focus),
+            cx.on_blur(&focus_handle, window, Self::on_blur),
+        ];
 
-        cx.on_focus(&focus_handle, window, Self::on_focus).detach();
+        Self {
-        cx.on_blur(&focus_handle, window, Self::on_blur).detach();
+            focus_handle: focus_handle.clone(),
-
+            length,
-        input
+            number_of_groups: 2,
+            value: SharedString::default(),
+            masked: false,
+            blink_cursor: blink_cursor.clone(),
+            size: Size::Medium,
+            _subscriptions,
+        }
     }
 
     /// Set number of groups in the OTP Input.

crates/ui/src/list/list.rs

@@ -13,7 +13,7 @@ use gpui::{
     ListSizingBehavior, MouseButton, ParentElement, Render, SharedString, Styled, Task,
     UniformListScrollHandle, Window,
 };
-use gpui::{px, App, Context, EventEmitter, ScrollStrategy};
+use gpui::{px, App, Context, EventEmitter, ScrollStrategy, Subscription};
 use smol::Timer;
 
 use super::loading::Loading;
@@ -158,6 +158,7 @@ pub struct List<D: ListDelegate> {
     right_clicked_index: Option<usize>,
     _search_task: Task<()>,
     _load_more_task: Task<()>,
+    _query_input_subscription: Subscription,
 }
 
 impl<D> List<D>
@@ -173,8 +174,8 @@ where
                 .cleanable()
         });
 
-        cx.subscribe_in(&query_input, window, Self::on_query_input_event)
+        let _query_input_subscription =
-            .detach();
+            cx.subscribe_in(&query_input, window, Self::on_query_input_event);
 
         Self {
             focus_handle: cx.focus_handle(),
@@ -192,6 +193,7 @@ where
             size: Size::default(),
             _search_task: Task::ready(()),
             _load_more_task: Task::ready(()),
+            _query_input_subscription,
         }
     }
 
@@ -233,8 +235,8 @@ where
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        cx.subscribe_in(&query_input, window, Self::on_query_input_event)
+        self._query_input_subscription =
-            .detach();
+            cx.subscribe_in(&query_input, window, Self::on_query_input_event);
         self.query_input = Some(query_input);
     }
 

crates/ui/src/notification.rs

@@ -1,9 +1,15 @@
-use std::{any::TypeId, collections::VecDeque, sync::Arc, time::Duration};
+use std::{
+    any::TypeId,
+    collections::{HashMap, VecDeque},
+    sync::Arc,
+    time::Duration,
+};
 
 use gpui::{
     div, prelude::FluentBuilder, px, Animation, AnimationExt, App, AppContext, ClickEvent, Context,
     DismissEvent, ElementId, Entity, EventEmitter, InteractiveElement as _, IntoElement,
-    ParentElement as _, Render, SharedString, StatefulInteractiveElement, Styled, Window,
+    ParentElement as _, Render, SharedString, StatefulInteractiveElement, Styled, Subscription,
+    Window,
 };
 use smol::Timer;
 
@@ -20,7 +26,7 @@ pub enum NotificationType {
     Error,
 }
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, PartialEq, Clone, Hash, Eq)]
 pub(crate) enum NotificationId {
     Id(TypeId),
     IdAndElementId(TypeId, ElementId),
@@ -286,6 +292,7 @@ pub struct NotificationList {
     /// Notifications that will be auto hidden.
     pub(crate) notifications: VecDeque<Entity<Notification>>,
     expanded: bool,
+    _subscriptions: HashMap<NotificationId, Subscription>,
 }
 
 impl NotificationList {
@@ -293,6 +300,7 @@ impl NotificationList {
         Self {
             notifications: VecDeque::new(),
             expanded: false,
+            _subscriptions: HashMap::new(),
         }
     }
 
@@ -310,10 +318,14 @@ impl NotificationList {
         self.notifications.retain(|note| note.read(cx).id != id);
 
         let notification = cx.new(|_| notification);
+
+        self._subscriptions.insert(
+            id.clone(),
             cx.subscribe(&notification, move |view, _, _: &DismissEvent, cx| {
                 view.notifications.retain(|note| id != note.read(cx).id);
-        })
+                view._subscriptions.remove(&id);
-        .detach();
+            }),
+        );
 
         self.notifications.push_back(notification.clone());
         if autohide {

crates/ui/src/popup_menu.rs

@@ -4,6 +4,7 @@ use crate::{
     button::Button, h_flex, list::ListItem, popover::Popover, v_flex, ActiveTheme, Icon, IconName,
     Selectable, Sizable as _,
 };
+use gpui::Subscription;
 use gpui::{
     actions, anchored, canvas, div, prelude::FluentBuilder, px, rems, Action, AnyElement, App,
     AppContext, Bounds, Context, Corner, DismissEvent, Edges, Entity, EventEmitter, FocusHandle,
@@ -107,7 +108,7 @@ pub struct PopupMenu {
     scroll_state: Rc<Cell<ScrollbarState>>,
 
     action_focus_handle: Option<FocusHandle>,
-    _subscriptions: [gpui::Subscription; 1],
+    _subscriptions: Vec<Subscription>,
 }
 
 impl PopupMenu {
@@ -118,10 +119,12 @@ impl PopupMenu {
     ) -> Entity<Self> {
         cx.new(|cx| {
             let focus_handle = cx.focus_handle();
-            let _on_blur_subscription =
+            let _subscriptions =
+                vec![
                     cx.on_blur(&focus_handle, window, |this: &mut PopupMenu, window, cx| {
                         this.dismiss(&Dismiss, window, cx)
-                });
+                    }),
+                ];
 
             let menu = Self {
                 focus_handle,
@@ -138,7 +141,7 @@ impl PopupMenu {
                 scrollable: false,
                 scroll_handle: ScrollHandle::default(),
                 scroll_state: Rc::new(Cell::new(ScrollbarState::default())),
-                _subscriptions: [_on_blur_subscription],
+                _subscriptions,
             };
             window.refresh();
             f(menu, window, cx)

crates/ui/src/time/date_picker.rs

@@ -3,7 +3,7 @@ use gpui::{
     anchored, deferred, div, prelude::FluentBuilder as _, px, App, AppContext, Context, ElementId,
     Entity, EventEmitter, FocusHandle, Focusable, InteractiveElement as _, KeyBinding, Length,
     MouseButton, ParentElement as _, Render, SharedString, StatefulInteractiveElement as _, Styled,
-    Window,
+    Subscription, Window,
 };
 use rust_i18n::t;
 
@@ -68,6 +68,7 @@ pub struct DatePicker {
     calendar: Entity<Calendar>,
     number_of_months: usize,
     presets: Option<Vec<DateRangePreset>>,
+    _subscriptions: Vec<Subscription>,
 }
 
 impl DatePicker {
@@ -103,7 +104,7 @@ impl DatePicker {
             this
         });
 
-        cx.subscribe_in(
+        let _subscriptions = vec![cx.subscribe_in(
             &calendar,
             window,
             |this, _, ev: &CalendarEvent, window, cx| match ev {
@@ -112,8 +113,7 @@ impl DatePicker {
                     this.focus_handle.focus(window);
                 }
             },
-        )
+        )];
-        .detach();
 
         Self {
             id: id.into(),
@@ -128,6 +128,7 @@ impl DatePicker {
             number_of_months: 1,
             placeholder: None,
             presets: None,
+            _subscriptions,
         }
     }