Merge pull request #450 from mahkoh/jorth/named-actions-loop

toml-config: clear named-action reference cycles

Cargo.lock

@@ -624,6 +624,7 @@ dependencies = [
  "error_reporter",
  "futures-util",
  "log",
+ "run-on-drop",
  "serde",
  "serde_json",
  "thiserror",

jay-config/Cargo.toml

@@ -17,3 +17,4 @@ backtrace = "0.3.69"
 error_reporter = "1.0.0"
 serde_json = "1.0.114"
 bstr = { version = "1.9.0", default-features = false, features = ["std"] }
+run-on-drop = "1.0.0"

jay-config/src/_private/client.rs

@@ -33,6 +33,7 @@ use {
     },
     bincode::Options,
     futures_util::task::ArcWake,
+    run_on_drop::{OnDrop, on_drop},
     std::{
         cell::{Cell, RefCell},
         collections::{HashMap, VecDeque, hash_map::Entry},
@@ -100,6 +101,7 @@ pub(crate) struct Client {
     on_del_drm_device: RefCell<Option<Callback<DrmDevice>>>,
     on_idle: RefCell<Option<Callback>>,
     on_switch_event: RefCell<HashMap<InputDevice, Callback<SwitchEvent>>>,
+    on_unload: Cell<Option<OnDrop<Box<dyn FnOnce()>>>>,
     bufs: RefCell<Vec<Vec<u8>>>,
     reload: Cell<bool>,
     read_interests: RefCell<HashMap<PollableId, Interest>>,
@@ -232,6 +234,7 @@ pub unsafe extern "C" fn init(
         on_del_drm_device: Default::default(),
         on_idle: Default::default(),
         on_switch_event: Default::default(),
+        on_unload: Default::default(),
         bufs: Default::default(),
         reload: Cell::new(false),
         read_interests: Default::default(),
@@ -1250,6 +1253,12 @@ impl Client {
         }
     }
 
+    pub fn on_unload(&self, f: impl FnOnce() + 'static) {
+        if let Some(prev) = self.on_unload.replace(Some(on_drop(Box::new(f)))) {
+            prev.forget();
+        }
+    }
+
     fn handle_msg(&self, msg: &[u8]) {
         self.handle_msg2(msg);
         self.dispatch_futures();

jay-config/src/lib.rs

@@ -304,3 +304,13 @@ pub fn toggle_float_above_fullscreen() {
 pub fn set_show_float_pin_icon(show: bool) {
     get!().set_show_float_pin_icon(show);
 }
+
+/// Sets a callback to run when this config is unloaded.
+///
+/// Only one callback can be set at a time. If another callback is already set, it will be
+/// dropped without being run.
+///
+/// This function can be used to terminate threads and clear reference cycles.
+pub fn on_unload(f: impl FnOnce() + 'static) {
+    get!().on_unload(f);
+}

toml-config/src/lib.rs

@@ -23,7 +23,7 @@ use {
         is_reload,
         keyboard::{Keymap, ModifiedKeySym},
         logging::set_log_level,
-        on_devices_enumerated, on_idle, quit, reload, set_color_management_enabled,
+        on_devices_enumerated, on_idle, on_unload, quit, reload, set_color_management_enabled,
         set_default_workspace_capture, set_explicit_sync_enabled, set_float_above_fullscreen,
         set_idle, set_idle_grace_period, set_show_float_pin_icon, set_ui_drag_enabled,
         set_ui_drag_threshold,
@@ -238,6 +238,7 @@ impl Action {
                 let name = Rc::new(name);
                 B::new(move || {
                     state
+                        .persistent
                         .actions
                         .borrow_mut()
                         .insert(name.clone(), action.clone());
@@ -246,7 +247,7 @@ impl Action {
             Action::UndefineAction { name } => {
                 let state = state.clone();
                 B::new(move || {
-                    state.actions.borrow_mut().remove(&name);
+                    state.persistent.actions.borrow_mut().remove(&name);
                 })
             }
             Action::NamedAction { name } => {
@@ -259,7 +260,7 @@ impl Action {
                     }
                     state.action_depth.set(depth + 1);
                     let _reset = on_drop(|| state.action_depth.set(depth));
-                    let Some(action) = state.actions.borrow().get(&name).cloned() else {
+                    let Some(action) = state.persistent.actions.borrow().get(&name).cloned() else {
                         log::error!("There is no action named {name}");
                         return;
                     };
@@ -648,7 +649,6 @@ impl Output {
     }
 }
 
-#[expect(clippy::type_complexity)]
 struct State {
     outputs: AHashMap<String, OutputMatch>,
     drm_devices: AHashMap<String, DrmDeviceMatch>,
@@ -662,7 +662,6 @@ struct State {
 
     action_depth_max: u64,
     action_depth: Cell<u64>,
-    actions: RefCell<AHashMap<Rc<String>, Rc<dyn Fn()>>>,
 }
 
 impl Drop for State {
@@ -882,6 +881,8 @@ struct PersistentState {
     default: Config,
     seat: Seat,
     binds: RefCell<AHashSet<ModifiedKeySym>>,
+    #[expect(clippy::type_complexity)]
+    actions: RefCell<AHashMap<Rc<String>, Rc<dyn Fn()>>>,
 }
 
 fn load_config(initial_load: bool, persistent: &Rc<PersistentState>) {
@@ -962,12 +963,12 @@ fn load_config(initial_load: bool, persistent: &Rc<PersistentState>) {
         io_outputs: Default::default(),
         action_depth_max: config.max_action_depth,
         action_depth: Cell::new(0),
-        actions: Default::default(),
     });
     state.set_status(&config.status);
+    persistent.actions.borrow_mut().clear();
     for a in config.named_actions {
         let action = a.action.into_rc_fn(&state);
-        state.actions.borrow_mut().insert(a.name, action);
+        persistent.actions.borrow_mut().insert(a.name, action);
     }
     let mut switch_actions = vec![];
     for input in &mut config.inputs {
@@ -1184,7 +1185,12 @@ pub fn configure() {
         default: default.unwrap(),
         seat: default_seat(),
         binds: Default::default(),
+        actions: Default::default(),
     });
+    {
+        let p = persistent.clone();
+        on_unload(move || p.actions.borrow_mut().clear());
+    }
     load_config(true, &persistent);
 }