mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
fix(app-server): speed up shutdown (#23578)
## Why Pressing `Ctrl+C` or `Ctrl+D` in the TUI could make Codex pause during shutdown when app-server background work still held outbound sender clones. Shutdown tracing against the current `~/.codex` path found three relevant holders: - `SkillsWatcher` kept its event-loop task alive until the shutdown timeout path. - `AppServerAttestationProvider` retained a strong `Arc<OutgoingMessageSender>`, which could keep outbound teardown waiting after the processor task had exited. - A background `apps/list` task could still own an outbound sender when shutdown began, causing the in-process app-server runtime to wait for its outbound channel to close. ## What Changed - Give `SkillsWatcher` an explicit shutdown `CancellationToken` and cancel it from app-server teardown so its event loop drops the outbound sender promptly. - Change `AppServerAttestationProvider` to keep a `Weak<OutgoingMessageSender>` and return immediately when it can no longer be upgraded. - Give `AppsRequestProcessor` a shutdown `CancellationToken` and cancel in-flight background `apps/list` work during teardown. ## How to Test 1. Start Codex TUI from a real home configuration. 2. Press `Ctrl+C`. 3. Confirm Codex exits promptly instead of pausing during shutdown. 4. Repeat with `Ctrl+D` and confirm the same prompt exit path. Focused manual trace validation from the investigation: - Before the full fix, reproduced shutdown traces showed outbound teardown waiting on lingering owners, including `attestation.provider=1` and later `apps.list.task=1`. - After the fix, fresh real-home `Ctrl+D` traces showed `app_server.runtime.outbound_state_after_processor_join` with `owners=none`, `app_server.runtime.wait_outbound_handle = 0ms`, and total TUI app-server shutdown around `18ms`. Targeted validation: - `RUST_MIN_STACK=8388608 cargo test -p codex-app-server`
This commit is contained in:
@@ -85,6 +85,7 @@ use tokio::sync::broadcast;
|
||||
use tokio::sync::watch;
|
||||
use tokio::time::Duration;
|
||||
use tokio::time::timeout;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::Instrument;
|
||||
|
||||
const EXTERNAL_AUTH_REFRESH_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
@@ -159,6 +160,7 @@ impl ExternalAuth for ExternalAuthRefreshBridge {
|
||||
|
||||
pub(crate) struct MessageProcessor {
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
skills_watcher: Arc<SkillsWatcher>,
|
||||
account_processor: AccountRequestProcessor,
|
||||
apps_processor: AppsRequestProcessor,
|
||||
catalog_processor: CatalogRequestProcessor,
|
||||
@@ -330,6 +332,7 @@ impl MessageProcessor {
|
||||
let thread_list_state_permit = Arc::new(Semaphore::new(/*permits*/ 1));
|
||||
let workspace_settings_cache =
|
||||
Arc::new(workspace_settings::WorkspaceSettingsCache::default());
|
||||
let app_list_shutdown_token = CancellationToken::new();
|
||||
let account_processor = AccountRequestProcessor::new(
|
||||
auth_manager.clone(),
|
||||
Arc::clone(&thread_manager),
|
||||
@@ -343,6 +346,7 @@ impl MessageProcessor {
|
||||
outgoing.clone(),
|
||||
config_manager.clone(),
|
||||
Arc::clone(&workspace_settings_cache),
|
||||
app_list_shutdown_token,
|
||||
);
|
||||
let catalog_processor = CatalogRequestProcessor::new(
|
||||
auth_manager.clone(),
|
||||
@@ -477,6 +481,7 @@ impl MessageProcessor {
|
||||
|
||||
Self {
|
||||
outgoing,
|
||||
skills_watcher,
|
||||
account_processor,
|
||||
apps_processor,
|
||||
catalog_processor,
|
||||
@@ -504,6 +509,8 @@ impl MessageProcessor {
|
||||
|
||||
pub(crate) fn clear_runtime_references(&self) {
|
||||
self.account_processor.clear_external_auth();
|
||||
self.apps_processor.shutdown();
|
||||
self.skills_watcher.shutdown();
|
||||
}
|
||||
|
||||
pub(crate) async fn process_request(
|
||||
|
||||
Reference in New Issue
Block a user