From b27356129f8e6222743c556cef3f65daeac0ae0b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 3 Nov 2026 05:48:35 +0000 Subject: [PATCH] fix: keep queued startup enable and disable commands Accepted startup toggles stay in order while the worker is busy. A later refresh no longer replaces that queue, and a toggle is not dropped when a refresh is already pending. Each command re-checks the stored entry identity immediately before the registry or startup-folder write. Co-authored-by: jameslincn --- src/pages/startup/mod.rs | 33 +++++--- src/pages/startup/model.rs | 157 ++++++++++++++++++++++++++++------- src/pages/startup/sampler.rs | 48 ++++++----- 3 files changed, 176 insertions(+), 62 deletions(-) diff --git a/src/pages/startup/mod.rs b/src/pages/startup/mod.rs index 6cf1847..38be0c9 100644 --- a/src/pages/startup/mod.rs +++ b/src/pages/startup/mod.rs @@ -15,7 +15,10 @@ mod sampler; // 启动项页列出 Run 键和“启动”文件夹中的项目，并在后台 worker 上启用或禁用。 // UI 线程只提交请求、维护列表选择和绘制。刷新保留来源加名称对应的选择、焦点和滚动位置。 // 某个来源枚举失败时保留该来源上一轮的行，并把这些行和页面提示标成未刷新。 +// 启用和禁用按接受顺序进入同一 worker 队列。后到的刷新不会丢掉尚未执行的切换， +// 已在等待的刷新也不会丢掉后到的切换。写入前用页面保存的身份再核对；对不上就跳过并报告原有失败。 // 启用和禁用不删除原项；写入结果读回不一致、项目已消失或 API 失败时立即显示错误。 +// 页面销毁时不在 UI 线程上 join 这个 worker。 use std::mem::zeroed; @@ -350,18 +353,20 @@ impl StartupPageState { if self.worker.is_some() { return Ok(()); } - self.worker = Some(SingleFlightWorker::spawn( + // Asynchronous retirement closes completion posts and does not join this worker + // on the UI thread, including while a queued toggle is still writing. + self.worker = Some(SingleFlightWorker::spawn_cancellable_initialized( "taskmgr-rs-startup-sampler", PWM_STARTUP_WORKER_COMPLETE, merge_startup_requests, - perform, + |_stop| perform, )?); Ok(()) } fn refresh(&mut self) { self.drain_worker(); - self.submit(StartupRequest::Refresh); + self.submit(StartupRequest::refresh()); } fn request_toggle(&mut self, toggle: StartupToggle) { @@ -378,14 +383,11 @@ impl StartupPageState { self.beep(); return; } - self.submit(StartupRequest::SetEnabled { id, toggle }); + self.submit(StartupRequest::toggle(id, toggle)); } fn submit(&mut self, request: StartupRequest) { - let toggle = match &request { - StartupRequest::SetEnabled { toggle, .. } => Some(*toggle), - StartupRequest::Refresh => None, - }; + let toggle = request.toggles.first().map(|command| command.toggle); let Some(worker) = self.worker.as_mut() else { self.report_submit_failure(StartupQueryFailure::Win32(ERROR_BROKEN_PIPE), toggle); return; @@ -432,12 +434,17 @@ impl StartupPageState { Ok(collection) => self.apply_collection(collection), Err(failure) => self.apply_refresh_failure(failure), }, - StartupWorkerCompletion::Action(report) => { - match report.outcome { - Ok(state) => self.apply_toggled_state(&report.id, state), - Err(failure) => self.show_action_failure(report.toggle, failure), + StartupWorkerCompletion::Actions(reports) => { + if reports.is_empty() { + return; + } + for report in reports { + match report.outcome { + Ok(state) => self.apply_toggled_state(&report.id, state), + Err(failure) => self.show_action_failure(report.toggle, failure), + } } - self.submit(StartupRequest::Refresh); + self.submit(StartupRequest::refresh()); } } } diff --git a/src/pages/startup/model.rs b/src/pages/startup/model.rs index 9c670ef..b9e2195 100644 --- a/src/pages/startup/model.rs +++ b/src/pages/startup/model.rs @@ -134,12 +134,35 @@ impl StartupToggle { } #[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) enum StartupRequest { - Refresh, - SetEnabled { - id: StartupEntryId, - toggle: StartupToggle, - }, +pub(crate) struct StartupToggleCommand { + pub(crate) id: StartupEntryId, + pub(crate) toggle: StartupToggle, +} + +/// One coalesced worker request. +/// +/// Enable and disable commands stay in [`StartupRequest::toggles`] in acceptance order. +/// `refresh` records that a snapshot is also wanted. Neither field replaces the other. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct StartupRequest { + pub(crate) toggles: Vec, + pub(crate) refresh: bool, +} + +impl StartupRequest { + pub(crate) fn refresh() -> Self { + Self { + toggles: Vec::new(), + refresh: true, + } + } + + pub(crate) fn toggle(id: StartupEntryId, toggle: StartupToggle) -> Self { + Self { + toggles: vec![StartupToggleCommand { id, toggle }], + refresh: false, + } + } } #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] @@ -271,6 +294,22 @@ pub(crate) fn classify_startup_result( } } +/// `stored` is the identity the page already kept for this command. +/// +/// The worker reads the live entry immediately before the registry or startup-folder write and +/// passes that entry here. A missing entry, or one that is no longer the same source and name, +/// is [`StartupActionFailure::Missing`]. The caller skips the write. +pub(crate) fn confirm_startup_identity( + stored: &StartupEntryId, + observed: Option<&StartupEntryId>, +) -> Result<(), StartupActionFailure> { + if observed.is_some_and(|observed| stored.matches(observed)) { + Ok(()) + } else { + Err(StartupActionFailure::Missing) + } +} + pub(crate) fn command_state(selected: Option) -> StartupCommandState { let Some(state) = selected else { return StartupCommandState::disabled(); @@ -281,12 +320,11 @@ pub(crate) fn command_state(selected: Option) -> StartupCom } } -/// Keeps a pending enable or disable ahead of a coalesced refresh. +/// Appends every accepted enable or disable. A refresh does not erase that queue, and a toggle +/// is not dropped when a refresh is already pending. pub(crate) fn merge_startup_requests(pending: &mut StartupRequest, incoming: StartupRequest) { - match (&*pending, &incoming) { - (StartupRequest::SetEnabled { .. }, StartupRequest::Refresh) => {} - _ => *pending = incoming, - } + pending.toggles.extend(incoming.toggles); + pending.refresh = pending.refresh || incoming.refresh; } pub(crate) const fn refresh_scroll_policy() -> StartupScrollPolicy { @@ -616,30 +654,91 @@ mod tests { ); } - #[test] - fn pending_toggle_survives_a_coalesced_refresh() { - let original = StartupRequest::SetEnabled { - id: id(StartupSource::CurrentUserRun, "OneDrive"), - toggle: StartupToggle::Disable, - }; - let mut pending = original.clone(); - merge_startup_requests(&mut pending, StartupRequest::Refresh); - assert_eq!(pending, original); + fn queued_toggle( + source: StartupSource, + name: &str, + toggle: StartupToggle, + ) -> StartupToggleCommand { + StartupToggleCommand { + id: id(source, name), + toggle, + } + } + #[test] + fn two_pending_toggles_survive_a_merge_and_a_refresh() { + let first = queued_toggle( + StartupSource::CurrentUserRun, + "OneDrive", + StartupToggle::Disable, + ); + let second = queued_toggle( + StartupSource::LocalMachineRun, + "SecurityHealth", + StartupToggle::Enable, + ); + let mut pending = StartupRequest::toggle(first.id.clone(), first.toggle); merge_startup_requests( &mut pending, - StartupRequest::SetEnabled { - id: id(StartupSource::LocalMachineRun, "SecurityHealth"), - toggle: StartupToggle::Enable, - }, + StartupRequest::toggle(second.id.clone(), second.toggle), ); - assert!(matches!( + assert_eq!( pending, - StartupRequest::SetEnabled { - toggle: StartupToggle::Enable, - .. + StartupRequest { + toggles: vec![first.clone(), second.clone()], + refresh: false, } - )); + ); + + merge_startup_requests(&mut pending, StartupRequest::refresh()); + assert_eq!( + pending, + StartupRequest { + toggles: vec![first.clone(), second], + refresh: true, + } + ); + + let mut refresh_first = StartupRequest::refresh(); + merge_startup_requests( + &mut refresh_first, + StartupRequest::toggle(first.id.clone(), first.toggle), + ); + assert_eq!( + refresh_first, + StartupRequest { + toggles: vec![first], + refresh: true, + } + ); + } + + #[test] + fn a_changed_startup_identity_is_the_existing_missing_failure() { + let stored = id(StartupSource::CurrentUserRun, "OneDrive"); + assert_eq!(confirm_startup_identity(&stored, Some(&stored)), Ok(())); + assert_eq!( + confirm_startup_identity( + &stored, + Some(&id(StartupSource::CurrentUserRun, "onedrive")) + ), + Ok(()) + ); + assert_eq!( + confirm_startup_identity(&stored, None), + Err(StartupActionFailure::Missing) + ); + assert_eq!( + confirm_startup_identity( + &stored, + Some(&id(StartupSource::LocalMachineRun, "OneDrive")) + ), + Err(StartupActionFailure::Missing) + ); + assert_eq!( + confirm_startup_identity(&stored, Some(&id(StartupSource::CurrentUserRun, "Teams"))), + Err(StartupActionFailure::Missing) + ); } #[test] diff --git a/src/pages/startup/sampler.rs b/src/pages/startup/sampler.rs index 5d2e30c..1d85a1f 100644 --- a/src/pages/startup/sampler.rs +++ b/src/pages/startup/sampler.rs @@ -14,8 +14,10 @@ //! The page worker owns every registry key and find handle. Enumeration reads the native //! 64-bit registry view, including the explicit WOW6432Node Run key. Enable and disable rewrite //! only the 12-byte StartupApproved value and then read it back, keeping the 2/3 family distinct -//! from the 6/7 family. The Run value and the Startup folder file are checked before and after -//! that write so a missing entry is never reported as a successful change. +//! from the 6/7 family. Queued commands run in acceptance order. Immediately before each write, +//! the Run value or startup file is read again and compared with the identity the page stored. +//! A missing or different entry skips that command and is [`StartupActionFailure::Missing`]. +//! The entry is checked again after the write so a disappearance is never a successful change. //! //! A source that fails to enumerate is listed in `omitted_sources` and contributes no rows. //! The page keeps the previous rows for that source. The snapshot is an error only when every @@ -51,9 +53,9 @@ use crate::infrastructure::native::{to_wide_null, widestr_ptr_to_string}; use super::model::{ StartupActionFailure, StartupEnabledState, StartupEntryId, StartupSource, StartupToggle, - approved_blob, approved_flag, classify_startup_result, executable_candidate, - has_version_resource_extension, is_absolute_win32_path, registry_value_name_is_safe, - startup_file_name_is_safe, state_from_approved, + approved_blob, approved_flag, classify_startup_result, confirm_startup_identity, + executable_candidate, has_version_resource_extension, is_absolute_win32_path, + registry_value_name_is_safe, startup_file_name_is_safe, state_from_approved, }; const VALUE_ENUM_LIMIT: u32 = 4_096; @@ -126,7 +128,7 @@ pub(super) struct StartupRefreshFailure { #[derive(Clone, Debug, PartialEq, Eq)] pub(super) enum StartupWorkerCompletion { Snapshot(Result), - Action(StartupActionReport), + Actions(Vec), } struct RegKey { @@ -185,14 +187,22 @@ impl SourcePart { } pub(super) fn perform(request: super::model::StartupRequest) -> StartupWorkerCompletion { - match request { - super::model::StartupRequest::Refresh => { - StartupWorkerCompletion::Snapshot(collect_startup_entries()) - } - super::model::StartupRequest::SetEnabled { id, toggle } => { - StartupWorkerCompletion::Action(set_enabled(&id, toggle)) - } - } + // Commands stay ahead of a coalesced snapshot. Each one re-checks the stored identity + // before it writes. The page requests the snapshot after these reports, so a refresh that + // arrived with the queue still runs, and the queue is not held inside enumeration. + if !request.toggles.is_empty() { + return StartupWorkerCompletion::Actions( + request + .toggles + .into_iter() + .map(|command| set_enabled(&command.id, command.toggle)) + .collect(), + ); + } + if request.refresh { + return StartupWorkerCompletion::Snapshot(collect_startup_entries()); + } + StartupWorkerCompletion::Actions(Vec::new()) } fn collect_startup_entries() -> Result { @@ -424,9 +434,8 @@ fn set_registry_enabled( if !registry_value_name_is_safe(&id.name) { return Err(StartupActionFailure::Win32(ERROR_INVALID_PARAMETER)); } - if !registry_entry_exists(id)? { - return Err(StartupActionFailure::Missing); - } + // The page stored this source and name. Re-read the Run value immediately before the write. + confirm_startup_identity(id, registry_entry_exists(id)?.then_some(id))?; let observed = write_and_read_approved(id, toggle)?; if !registry_entry_exists(id)? { return Err(StartupActionFailure::Missing); @@ -442,9 +451,8 @@ fn set_folder_enabled( return Err(StartupActionFailure::Win32(ERROR_INVALID_PARAMETER)); } let path = startup_file_path(id)?; - if !startup_file_exists(&path)? { - return Err(StartupActionFailure::Missing); - } + // The page stored this source and name. Re-read the startup file immediately before the write. + confirm_startup_identity(id, startup_file_exists(&path)?.then_some(id))?; let observed = write_and_read_approved(id, toggle)?; if !startup_file_exists(&path)? { return Err(StartupActionFailure::Missing);