Move Windows dialogs and credential I/O off the foreground thread (#64250)

## Summary

Move Windows file-open, file-save, and native message dialogs off the
foreground executor so time spent waiting for a user does not appear as
a long foreground task poll. Move Windows Credential Manager reads,
writes, and deletes to the background executor as well.

The caller-facing receiver/task APIs remain unchanged. No hang-journal
suppression or new public GPUI task-spawn API is introduced.

## Implementation

- Run synchronous native dialogs on dedicated threads with COM STA
initialization and cleanup on the same thread.
- Keep the native owner relationship when a window exists; support
ownerless file/save dialogs by passing a null owner HWND.
- Serialize dialogs per owner using one mutex, also used to wait for
active native work before destroying the owner HWND. Closing the GPUI
window sets a shared cancellation flag and hides the native window;
queued requests see that flag and cannot launch. No pending counter,
lifetime lease, cancellation registry, or idle-notification channels are
needed.
- Use a worker-thread timer to request native cancellation when the
caller drops its receiver or the owner closes.
- Preserve message prompts without a Cancel option by vetoing user
cancellation unless an internal cancellation was requested.
- Distinguish actual file-dialog failures from user cancellation.

Sleep/wake and frame-visibility telemetry are intentionally deferred.

## Validation

- `corgi fmt -p gpui_windows -- --check` and scoped `git diff --check`
passed.
- Minimal scratch Windows-target harnesses type-checked the dialog
module and native API wiring against real Windows 0.62 bindings. These
are not full integration builds.
- Six extracted native-independent state/policy tests passed, covering
owner shutdown, queued-request cancellation state, per-request
cancellation isolation, per-owner serialization, ownerless cancellation,
and 12 message-prompt cancellation-policy combinations. These do not
execute native dialogs or the complete coordinator/worker handoff.
- Before the shutdown simplification, Cameron built and launched Zed
successfully on Windows using a shorter `CARGO_TARGET_DIR`, and reported
running the manual prompt checks. The captured output contains no
foreground/action hang or saved task-trace messages, but exact
prompt-by-prompt coverage and timing have not been established.
- The committed Windows test suite has not yet been run natively.

## Open review items

This PR remains a draft. The dialog helper has been simplified to
approximately 245 production lines, retaining native ownership and using
the existing per-owner mutex for shutdown instead of separate lifetime
bookkeeping. Independent static review found no concrete lifetime
regression. Native cancellation still uses the existing worker-thread
timer/window-enumeration mechanism; Windows runtime validation of the
revised shutdown path is outstanding.

Before merging, verify ownerless prompts, focus/Alt+Tab behavior,
overlapping requests for the same owner, cancellation while
queued/shown, owner closure, and nested overwrite confirmations on
Windows. Absence of hang logs alone does not establish those behaviors.

The required review-confirmation marker in README.md must be removed
manually by the human author after review.

Release Notes:

- N/A
- [GPUI] Move window dialogs and credential I/O off foreground thread
This commit is contained in:
Anthony Eid authored and GitHub committed 2026-09-15 18:59:14 +00:00
1 parent bec63dee23
commit eb655653f2
4 files changed
+401 -42

No files matched your search

+340
View File
@@ -0,0 +1,340 @@
use std::{
cell::RefCell,
rc::Rc,
sync::{
Arc,
atomic::{AtomicBool, Ordering},
},
};
use anyhow::{Context as _, Result, anyhow};
use futures::{
channel::oneshot,
future::{Either, select},
lock::Mutex,
};
use gpui::ForegroundExecutor;
use gpui_util::ResultExt;
use windows::{
Win32::{
Foundation::*,
System::{Com::*, Threading::GetCurrentThreadId},
UI::{
Controls::{TASKDIALOG_NOTIFICATIONS, TDN_BUTTON_CLICKED},
WindowsAndMessaging::*,
},
},
core::{BOOL, HRESULT, w},
};
use crate::SafeHwnd;
pub(crate) struct DialogOwner {
hwnd: HWND,
closed: Arc<AtomicBool>,
serialization: Mutex<()>,
}
impl DialogOwner {
pub(crate) fn new(hwnd: HWND) -> Rc<Self> {
Rc::new(Self {
hwnd,
closed: Arc::new(AtomicBool::new(false)),
serialization: Mutex::new(()),
})
}
pub(crate) fn close(&self) {
self.closed.store(true, Ordering::Release);
}
pub(crate) async fn when_idle(&self) {
// After close(), queued requests cannot start a native dialog. Only the
// active dialog can still use the HWND, and it holds this lock until exit.
let _guard = self.serialization.lock().await;
}
}
#[derive(Clone, Default)]
struct Cancellation {
requested: Arc<AtomicBool>,
owner_closed: Option<Arc<AtomicBool>>,
}
impl Cancellation {
fn new(owner: Option<&DialogOwner>) -> Self {
Self {
owner_closed: owner.map(|owner| owner.closed.clone()),
..Self::default()
}
}
fn is_requested(&self) -> bool {
self.requested.load(Ordering::Acquire)
|| self
.owner_closed
.as_ref()
.is_some_and(|closed| closed.load(Ordering::Acquire))
}
}
/// Native modal loops may synchronously message their owner, so the foreground
/// thread must keep pumping messages rather than waiting for the dialog thread.
/// Holding the owner's lock through native completion also allows window
/// destruction to wait for the dialog without blocking the foreground thread.
pub(crate) fn show_dialog<T: Send + 'static>(
owner: Option<Rc<DialogOwner>>,
executor: &ForegroundExecutor,
dialog: impl FnOnce(HWND) -> Result<T> + Send + 'static,
) -> oneshot::Receiver<Result<T>> {
let (mut sender, receiver) = oneshot::channel();
let cancellation = Cancellation::new(owner.as_deref());
if cancellation.is_requested() {
sender.send(Err(anyhow!("dialog owner has closed"))).ok();
return receiver;
}
let hwnd = owner.as_ref().map(|owner| SafeHwnd::from(owner.hwnd));
executor
.spawn(async move {
// Let Windows manage enabling and activating the owner. Overlapping
// modal loops could otherwise re-enable it while another dialog is open.
let serialization = if let Some(owner) = owner.as_ref() {
match select(owner.serialization.lock(), sender.cancellation()).await {
Either::Left((guard, _)) => Some(guard),
Either::Right(((), _)) => return,
}
} else {
None
};
if sender.is_canceled() || cancellation.is_requested() {
return;
}
let (result_sender, mut result_receiver) = oneshot::channel();
let thread = std::thread::Builder::new()
.name("windows-native-dialog".into())
.spawn({
let cancellation = cancellation.clone();
move || {
let result = (|| {
let _apartment = Apartment::new()?;
let _timer = CancellationTimer::new(cancellation.clone())?;
if cancellation.is_requested() {
return Err(anyhow!("dialog cancelled"));
}
let result = dialog(hwnd.map(|hwnd| hwnd.as_raw()).unwrap_or_default());
if cancellation.is_requested() {
return Err(anyhow!("dialog cancelled"));
}
result
})();
result_sender.send(result).ok();
}
});
if let Err(error) = thread {
drop(serialization);
sender.send(Err(error.into())).ok();
return;
}
let result = match select(&mut result_receiver, sender.cancellation()).await {
Either::Left((result, _)) => result,
Either::Right(((), _)) => {
cancellation.requested.store(true, Ordering::Release);
result_receiver.await
}
};
// Neither launch another dialog nor destroy the owner until the
// native loop exits, even if the caller no longer wants its result.
drop(serialization);
sender
.send(result.unwrap_or_else(|error| Err(error.into())))
.ok();
})
.detach();
receiver
}
struct Apartment;
impl Apartment {
fn new() -> Result<Self> {
unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED).ok()? };
Ok(Self)
}
}
impl Drop for Apartment {
fn drop(&mut self) {
unsafe { CoUninitialize() };
}
}
thread_local! {
static CANCELLATION: RefCell<Option<Cancellation>> = const { RefCell::new(None) };
}
struct CancellationTimer(usize);
impl CancellationTimer {
fn new(cancellation: Cancellation) -> Result<Self> {
let timer = unsafe { SetTimer(None, 0, 100, Some(cancel_dialog)) };
if timer == 0 {
return Err(windows::core::Error::from_thread())
.context("starting native dialog cancellation timer");
}
CANCELLATION.with(|slot| slot.replace(Some(cancellation)));
Ok(Self(timer))
}
}
impl Drop for CancellationTimer {
fn drop(&mut self) {
CANCELLATION.with(|slot| slot.take());
unsafe { KillTimer(None, self.0).log_err() };
}
}
unsafe extern "system" fn cancel_dialog(_: HWND, _: u32, _: usize, _: u32) {
if cancellation_requested() {
unsafe {
EnumThreadWindows(GetCurrentThreadId(), Some(close_dialog), LPARAM(0))
.ok()
.log_err();
};
}
}
fn cancellation_requested() -> bool {
CANCELLATION.with(|slot| {
slot.borrow()
.as_ref()
.is_some_and(Cancellation::is_requested)
})
}
pub(crate) unsafe extern "system" fn task_dialog_callback(
_: HWND,
notification: TASKDIALOG_NOTIFICATIONS,
button: WPARAM,
_: LPARAM,
has_cancel_button: isize,
) -> HRESULT {
// WM_CLOSE requires TDF_ALLOW_DIALOG_CANCELLATION, which also enables Esc/X.
// Only allow those user gestures when the prompt offered a Cancel button.
if notification == TDN_BUTTON_CLICKED
&& button.0 == IDCANCEL.0 as usize
&& has_cancel_button == 0
&& !cancellation_requested()
{
S_FALSE
} else {
S_OK
}
}
unsafe extern "system" fn close_dialog(hwnd: HWND, _: LPARAM) -> BOOL {
// Shell dialogs can open further modal dialogs. Close only dialog windows
// on this dedicated thread, not hidden COM infrastructure windows.
let mut class_name = [0u16; 32];
let length = unsafe { GetClassNameW(hwnd, &mut class_name) } as usize;
let dialog_class = w!("#32770");
if class_name.get(..length) == Some(unsafe { dialog_class.as_wide() }) {
unsafe { PostMessageW(Some(hwnd), WM_CLOSE, WPARAM(0), LPARAM(0)).log_err() };
}
TRUE
}
#[cfg(test)]
mod tests {
use super::*;
use futures::FutureExt as _;
#[test]
fn task_dialog_cancellation_requires_cancel_button_or_internal_request() {
let cancellation = Cancellation::default();
CANCELLATION.with(|slot| slot.replace(Some(cancellation.clone())));
for internally_cancelled in [false, true] {
cancellation
.requested
.store(internally_cancelled, Ordering::Release);
for has_cancel_button in [false, true] {
for button in [IDCANCEL.0, IDOK.0, -1] {
let result = unsafe {
task_dialog_callback(
HWND::default(),
TDN_BUTTON_CLICKED,
WPARAM(button as usize),
LPARAM(0),
has_cancel_button as isize,
)
};
let should_veto =
button == IDCANCEL.0 && !has_cancel_button && !internally_cancelled;
assert_eq!(result, if should_veto { S_FALSE } else { S_OK });
}
}
}
CANCELLATION.with(|slot| slot.take());
}
#[test]
fn closing_owner_cancels_every_outstanding_dialog() {
let owner = DialogOwner::new(HWND::default());
let first = Cancellation::new(Some(&owner));
let second = Cancellation::new(Some(&owner));
assert!(!first.is_requested());
assert!(!second.is_requested());
owner.close();
owner.close();
assert!(first.is_requested());
assert!(second.is_requested());
assert!(Cancellation::new(Some(&owner)).is_requested());
}
#[test]
fn closed_owner_waits_for_active_dialog_but_queued_requests_stay_cancelled() {
let owner = DialogOwner::new(HWND::default());
let active = owner.serialization.try_lock().expect("first dialog");
let queued = Cancellation::new(Some(&owner));
owner.close();
assert!(owner.when_idle().now_or_never().is_none());
drop(active);
assert!(owner.when_idle().now_or_never().is_some());
let _guard = owner.serialization.try_lock().expect("queued request");
assert!(queued.is_requested());
}
#[test]
fn cancelling_one_request_does_not_cancel_its_owner_or_other_requests() {
let owner = DialogOwner::new(HWND::default());
let first = Cancellation::new(Some(&owner));
let second = Cancellation::new(Some(&owner));
first.requested.store(true, Ordering::Release);
assert!(first.is_requested());
assert!(!second.is_requested());
assert!(!owner.closed.load(Ordering::Acquire));
}
#[test]
fn dialogs_serialize_per_owner() {
let owner = DialogOwner::new(HWND::default());
let other_owner = DialogOwner::new(HWND::default());
let guard = owner.serialization.try_lock().expect("first dialog");
assert!(owner.serialization.try_lock().is_none());
assert!(other_owner.serialization.try_lock().is_some());
drop(guard);
assert!(owner.serialization.try_lock().is_some());
}
#[test]
fn ownerless_request_can_be_cancelled_without_a_window() {
let cancellation = Cancellation::new(None);
assert!(!cancellation.is_requested());
cancellation.requested.store(true, Ordering::Release);
assert!(cancellation.is_requested());
}
}
+1
View File
@@ -2,6 +2,7 @@
mod clipboard;
mod destination_list;
mod dialog;
mod direct_manipulation;
mod direct_write;
mod directx_atlas;
+27 -27
View File
@@ -674,15 +674,13 @@ impl Platform for WindowsPlatform {
&self,
options: PathPromptOptions,
) -> Receiver<Result<Option<Vec<PathBuf>>>> {
let (tx, rx) = oneshot::channel();
let window = self.find_current_active_window();
self.foreground_executor()
.spawn(async move {
let _ = tx.send(file_open_dialog(options, window));
})
.detach();
rx
let owner = self
.find_current_active_window()
.and_then(|hwnd| self.window_from_hwnd(hwnd))
.map(|window| window.dialog_owner.clone());
crate::dialog::show_dialog(owner, &self.foreground_executor, move |window| {
file_open_dialog(options, Some(window))
})
}
fn prompt_for_new_path(
@@ -692,15 +690,13 @@ impl Platform for WindowsPlatform {
) -> Receiver<Result<Option<PathBuf>>> {
let directory = directory.to_owned();
let suggested_name = suggested_name.map(|s| s.to_owned());
let (tx, rx) = oneshot::channel();
let window = self.find_current_active_window();
self.foreground_executor()
.spawn(async move {
let _ = tx.send(file_save_dialog(directory, suggested_name, window));
})
.detach();
rx
let owner = self
.find_current_active_window()
.and_then(|hwnd| self.window_from_hwnd(hwnd))
.map(|window| window.dialog_owner.clone());
crate::dialog::show_dialog(owner, &self.foreground_executor, move |window| {
file_save_dialog(directory, suggested_name, Some(window))
})
}
fn can_select_mixed_files_and_dirs(&self) -> bool {
@@ -918,7 +914,7 @@ impl Platform for WindowsPlatform {
.encode_utf16()
.chain(Some(0))
.collect_vec();
self.foreground_executor().spawn(async move {
self.background_executor().spawn(async move {
let credentials = CREDENTIALW {
LastWritten: unsafe { GetSystemTimeAsFileTime() },
Flags: CRED_FLAGS(0),
@@ -947,7 +943,7 @@ impl Platform for WindowsPlatform {
.encode_utf16()
.chain(Some(0))
.collect_vec();
self.foreground_executor().spawn(async move {
self.background_executor().spawn(async move {
let mut credentials: *mut CREDENTIALW = std::ptr::null_mut();
let result = unsafe {
CredReadW(
@@ -989,7 +985,7 @@ impl Platform for WindowsPlatform {
.encode_utf16()
.chain(Some(0))
.collect_vec();
self.foreground_executor().spawn(async move {
self.background_executor().spawn(async move {
unsafe {
CredDeleteW(
PCWSTR::from_raw(target_name.as_ptr()),
@@ -1398,9 +1394,11 @@ fn file_open_dialog(
folder_dialog.SetOkButtonLabel(&HSTRING::from(prompt))?;
}
if folder_dialog.Show(window).is_err() {
// User cancelled
return Ok(None);
if let Err(error) = folder_dialog.Show(window) {
if error.code() == HRESULT::from_win32(ERROR_CANCELLED.0) {
return Ok(None);
}
return Err(error.into());
}
}
@@ -1458,9 +1456,11 @@ fn file_save_dialog(
pszName: windows::core::w!("All files"),
pszSpec: windows::core::w!("*.*"),
}])?;
if dialog.Show(window).is_err() {
// User cancelled
return Ok(None);
if let Err(error) = dialog.Show(window) {
if error.code() == HRESULT::from_win32(ERROR_CANCELLED.0) {
return Ok(None);
}
return Err(error.into());
}
}
let shell_item = unsafe { dialog.GetResult()? };
+33 -15
View File
@@ -93,6 +93,7 @@ pub struct WindowsWindowState {
pub(crate) struct WindowsWindowInner {
hwnd: HWND,
pub(crate) dialog_owner: Rc<crate::dialog::DialogOwner>,
drop_target_helper: IDropTargetHelper,
pub(crate) state: WindowsWindowState,
system_settings: WindowsSystemSettings,
@@ -281,6 +282,7 @@ impl WindowsWindowInner {
Ok(Rc::new(Self {
hwnd,
dialog_owner: crate::dialog::DialogOwner::new(hwnd),
drop_target_helper: context.drop_target_helper.clone(),
state,
handle: context.handle,
@@ -605,6 +607,8 @@ impl rwh::HasDisplayHandle for WindowsWindow {
impl Drop for WindowsWindow {
fn drop(&mut self) {
self.0.dialog_owner.close();
unsafe { ShowWindowAsync(self.0.hwnd, SW_HIDE).ok().log_err() };
// `DestroyWindow` below sends `WM_SHOWWINDOW`; without a callback the
// resulting visibility report has nothing to notify.
self.0.state.callbacks.visibility_change.take();
@@ -613,6 +617,7 @@ impl Drop for WindowsWindow {
self.0
.executor
.spawn(async move {
this.dialog_owner.when_idle().await;
let handle = this.hwnd;
unsafe {
RevokeDragDrop(handle).log_err();
@@ -717,18 +722,19 @@ impl PlatformWindow for WindowsWindow {
detail: Option<&str>,
answers: &[PromptButton],
) -> Option<Receiver<usize>> {
let (done_tx, done_rx) = oneshot::channel();
let (mut done_tx, done_rx) = oneshot::channel();
let msg = msg.to_string();
let detail_string = detail.map(|detail| detail.to_string());
let handle = self.0.hwnd;
let answers = answers.to_vec();
self.0
.executor
.spawn(async move {
let dialog = crate::dialog::show_dialog(
Some(self.0.dialog_owner.clone()),
&self.0.executor,
move |handle| {
unsafe {
let mut config = TASKDIALOGCONFIG::default();
config.cbSize = std::mem::size_of::<TASKDIALOGCONFIG>() as _;
config.hwndParent = handle;
config.dwFlags = TDF_ALLOW_DIALOG_CANCELLATION;
let title;
let main_icon;
match level {
@@ -776,17 +782,26 @@ impl PlatformWindow for WindowsWindow {
config.cButtons = buttons.len() as _;
config.pButtons = buttons.as_ptr();
config.pfCallback = None;
config.pfCallback = Some(crate::dialog::task_dialog_callback);
config.lpCallbackData = button_id_map.contains(&IDCANCEL.0) as isize;
let mut res = std::mem::zeroed();
let _ = TaskDialogIndirect(&config, Some(&mut res), None, None)
.context("unable to create task dialog")
.log_err();
if let Some(clicked) =
button_id_map.iter().position(|&button_id| button_id == res)
{
let _ = done_tx.send(clicked);
}
TaskDialogIndirect(&config, Some(&mut res), None, None)
.context("unable to create task dialog")?;
Ok(button_id_map.iter().position(|&button_id| button_id == res))
}
},
);
self.0
.executor
.spawn(async move {
if let futures::future::Either::Left((result, _)) =
futures::future::select(dialog, done_tx.cancellation()).await
&& let Some(Some(clicked)) = result
.context("native dialog thread stopped")
.and_then(|result| result)
.log_err()
{
done_tx.send(clicked).ok();
}
})
.detach();
@@ -1447,6 +1462,9 @@ unsafe extern "system" fn window_procedure(
}
let inner = unsafe { &*ptr };
let result = if let Some(inner) = inner.upgrade() {
if msg == WM_NCDESTROY {
inner.dialog_owner.close();
}
inner.handle_msg(hwnd, msg, wparam, lparam)
} else {
unsafe { DefWindowProcW(hwnd, msg, wparam, lparam) }