From 9ce4c2891e23ce77bd8835d57eb53d4a818a57f8 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Thu, 23 Jan 2025 15:58:11 -0800 Subject: [PATCH] explicitly activate and deactivate account relay list subs - ensures only one active at a time - stops leaking relay list subs --- crates/notedeck/src/accounts.rs | 104 +++++++++++++++++++++++--------- crates/notedeck/src/app.rs | 2 +- 2 files changed, 77 insertions(+), 29 deletions(-) diff --git a/crates/notedeck/src/accounts.rs b/crates/notedeck/src/accounts.rs index 818874e..a44f0ca 100644 --- a/crates/notedeck/src/accounts.rs +++ b/crates/notedeck/src/accounts.rs @@ -37,7 +37,7 @@ pub enum AccountsAction { pub struct AccountRelayData { filter: Filter, - subid: String, + subid: Option, sub: Option, local: BTreeSet, // used locally but not advertised advertised: BTreeSet, // advertised via NIP-65 @@ -56,7 +56,7 @@ pub struct AddAccountAction { } impl AccountRelayData { - pub fn new(ndb: &Ndb, pool: &mut RelayPool, pubkey: &[u8; 32]) -> Self { + pub fn new(ndb: &Ndb, pubkey: &[u8; 32]) -> Self { // Construct a filter for the user's NIP-65 relay list let filter = Filter::new() .authors([pubkey]) @@ -64,11 +64,6 @@ impl AccountRelayData { .limit(1) .build(); - // Local ndb subscription - let ndbsub = ndb - .subscribe(&[filter.clone()]) - .expect("ndb relay list subscription"); - // Query the ndb immediately to see if the user list is already there let txn = Transaction::new(ndb).expect("transaction"); let lim = filter.limit().unwrap_or(crate::filter::default_limit()) as i32; @@ -85,21 +80,51 @@ impl AccountRelayData { relays ); - // Id for future remote relay subscriptions - let subid = Uuid::new_v4().to_string(); - - // Add remote subscription to existing relays - pool.subscribe(subid.clone(), vec![filter.clone()]); - AccountRelayData { filter, - subid, - sub: Some(ndbsub), + subid: None, + sub: None, local: BTreeSet::new(), advertised: relays.into_iter().collect(), } } + // make this account the current selected account + pub fn activate(&mut self, ndb: &Ndb, pool: &mut RelayPool) { + debug!("activating relay sub {}", self.filter.json().unwrap()); + assert_eq!(self.subid, None, "subid already exists"); + assert_eq!(self.sub, None, "sub already exists"); + + // local subscription + let sub = ndb + .subscribe(&[self.filter.clone()]) + .expect("ndb relay list subscription"); + + // remote subscription + let subid = Uuid::new_v4().to_string(); + pool.subscribe(subid.clone(), vec![self.filter.clone()]); + + self.sub = Some(sub); + self.subid = Some(subid); + } + + // this account is no longer the selected account + pub fn deactivate(&mut self, ndb: &mut Ndb, pool: &mut RelayPool) { + debug!("deactivating relay sub {}", self.filter.json().unwrap()); + assert_ne!(self.subid, None, "subid doesn't exist"); + assert_ne!(self.sub, None, "sub doesn't exist"); + + // remote subscription + pool.unsubscribe(self.subid.as_ref().unwrap().clone()); + + // local subscription + ndb.unsubscribe(self.sub.unwrap()) + .expect("ndb relay list unsubscribe"); + + self.sub = None; + self.subid = None; + } + // standardize the format (ie, trailing slashes) to avoid dups pub fn canonicalize_url(url: &str) -> String { match Url::parse(url) { @@ -432,13 +457,12 @@ impl Accounts { } } - pub fn get_selected_account_data(&self) -> Option<&AccountData> { - if let Some(account) = self.get_selected_account() { - if let Some(account_data) = self.account_data.get(account.pubkey.bytes()) { - return Some(account_data); - } - } - None + pub fn get_selected_account_data(&mut self) -> Option<&mut AccountData> { + let account_pubkey = { + let account = self.get_selected_account()?; + *account.pubkey.bytes() + }; + self.account_data.get_mut(&account_pubkey) } pub fn select_account(&mut self, index: usize) { @@ -470,10 +494,13 @@ impl Accounts { pub fn send_initial_filters(&mut self, pool: &mut RelayPool, relay_url: &str) { for data in self.account_data.values() { - pool.send_to( - &ClientMessage::req(data.relay.subid.clone(), vec![data.relay.filter.clone()]), - relay_url, - ); + // send the active account's relay list subscription + if let Some(relay_subid) = &data.relay.subid { + pool.send_to( + &ClientMessage::req(relay_subid.clone(), vec![data.relay.filter.clone()]), + relay_url, + ); + } pool.send_to( &ClientMessage::req(data.muted.subid.clone(), vec![data.muted.filter.clone()]), relay_url, @@ -504,7 +531,7 @@ impl Accounts { // Create the user account data let new_account_data = AccountData { - relay: AccountRelayData::new(ndb, pool, pubkey), + relay: AccountRelayData::new(ndb, pubkey), muted: AccountMutedData::new(ndb, pool, pubkey), }; self.account_data.insert(*pubkey, new_account_data); @@ -602,7 +629,7 @@ impl Accounts { debug!("current relays: {:?}", pool.urls()); } - pub fn update(&mut self, ndb: &Ndb, pool: &mut RelayPool, ctx: &egui::Context) { + pub fn update(&mut self, ndb: &mut Ndb, pool: &mut RelayPool, ctx: &egui::Context) { // IMPORTANT - This function is called in the UI update loop, // make sure it is fast when idle @@ -614,6 +641,19 @@ impl Accounts { ctx2.request_repaint(); }; + // Do we need to deactivate any existing account subs? + for (ndx, account) in self.accounts.iter().enumerate() { + if Some(ndx) != self.currently_selected_account { + // this account is not currently selected + if let Some(data) = self.account_data.get_mut(account.pubkey.bytes()) { + if data.relay.sub.is_some() { + // this account has subs, deactivate them + data.relay.deactivate(ndb, pool); + } + } + } + } + // Were any accounts added or removed? let (added, removed) = self.delta_accounts(); for pk in added { @@ -633,6 +673,14 @@ impl Accounts { self.update_relay_configuration(pool, wakeup); self.needs_relay_config = false; } + + // Do we need to activate account subs? + if let Some(data) = self.get_selected_account_data() { + if data.relay.sub.is_none() { + // the currently selected account doesn't have subs, activate them + data.relay.activate(ndb, pool); + } + } } pub fn get_full<'a>(&'a self, pubkey: &[u8; 32]) -> Option> { diff --git a/crates/notedeck/src/app.rs b/crates/notedeck/src/app.rs index c4169fc..4f34ba3 100644 --- a/crates/notedeck/src/app.rs +++ b/crates/notedeck/src/app.rs @@ -69,7 +69,7 @@ impl eframe::App for Notedeck { puffin::GlobalProfiler::lock().new_frame(); // handle account updates - self.accounts.update(&self.ndb, &mut self.pool, ctx); + self.accounts.update(&mut self.ndb, &mut self.pool, ctx); main_panel(&ctx.style(), crate::ui::is_narrow(ctx)).show(ctx, |ui| { // render app