Skip to content

Prepare side threads off the TUI event loop to prevent deadlock - #26754

Closed
etraut-openai wants to merge 9 commits into
mainfrom
etraut/side-thread-nonblocking-prepare
Closed

Prepare side threads off the TUI event loop to prevent deadlock#26754
etraut-openai wants to merge 9 commits into
mainfrom
etraut/side-thread-nonblocking-prepare

Conversation

@etraut-openai

@etraut-openai etraut-openai commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

This PR fixes a deadlock that can occur when the TUI prepares a side conversation. Specifically, this can occur if the main thread generates many events in a short period of time when the side preparation takes a long time because of a slow fork operation.

Previously, /side waited for the fork and boundary-injection requests on the main event loop. App-server responses and notifications share a bounded transport, so a notification backlog could block the fork response while the TUI was no longer draining notifications. The backend would keep running, but the TUI could stop rendering and reading input permanently.

Changes

  • Run the side-thread fork and boundary injection in the background so the TUI continues to process app-server notifications and input events.
  • Switch to the prepared side thread using the returned local snapshot, avoiding another app-server round trip on the TUI event loop.
  • Make the composer temporarily read-only while preparation is in progress.
  • Allow the user to cancel a long-running preparation with Esc, restore the pending message, and clean up the unused side thread if the fork completes afterward.

Verification

  • Added coverage that cancelling side-thread preparation restores the pending message.
  • Updated the /side TUI snapshot to cover the read-only composer and cancellation hint.

@etraut-openai
etraut-openai marked this pull request as ready for review June 6, 2026 07:19
@etraut-openai etraut-openai changed the title [codex] Prepare side threads off the TUI event loop Fix TUI deadlock while preparing side conversations Jun 6, 2026
@etraut-openai etraut-openai changed the title Fix TUI deadlock while preparing side conversations Prepare side threads off the TUI event loop Jun 6, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e6d6bdd589

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/tui/src/app/side.rs Outdated
Comment thread codex-rs/tui/src/app/side.rs Outdated
Comment thread codex-rs/tui/src/app/side_server.rs Outdated
@openai openai deleted a comment from Bnilesh0210 Jun 6, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6adf62f75c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/tui/src/app/side.rs Outdated
Comment thread codex-rs/tui/src/app/side.rs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e5b9dc45d0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/tui/src/app/side_server.rs Outdated
Comment thread codex-rs/tui/src/app_event.rs Outdated
Comment thread codex-rs/tui/src/app/side.rs
Comment thread codex-rs/tui/src/app/side.rs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1664555828

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/tui/src/app/side.rs Outdated
Comment thread codex-rs/tui/src/app/side.rs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e748bdced2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/tui/src/app/side_server.rs Outdated
@etraut-openai etraut-openai changed the title Prepare side threads off the TUI event loop Prepare side threads off the TUI event loop to prevent deadlock Jun 9, 2026

@fcoury-oai fcoury-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got a hard crash. I started codex in this branch with just c. Then I typed:

Run exactly this command and wait for it to finish:
fish -c 'for i in (seq 1 600); echo "event-$i"; sleep 0.02; end'

And then:

/side Summarize what the main thread is currently doing.

It showed Preparing side conversation... Press Esc to cancel. briefly, cleared the screen and crashed.

Image

After a bit it recovered?

Image

I started it again with full backtrace tracking:

Image

Diagnostics by codex


The new nonblocking /side path exposes an interrupted-fork history integrity bug:

  1. The parent is mid-tool-call, so persisted history contains CustomToolCall(call_id) but not its output yet.
  2. thread/fork reads that partial persisted history in thread_processor.rs.
  3. Core forks it with ForkSnapshot::Interrupted, but thread_manager.rs only appends the interruption marker and TurnAborted. It does not close dangling tool calls.
  4. The side question starts a new turn using history containing the unmatched call.
  5. normalize.rs detects the missing CustomToolCallOutput and deliberately panics in debug builds, matching the reported call ID and stack exactly.

This is a pre-existing core interrupted-fork defect made reliably reachable by the PR’s background fork in side_server.rs. It is not caused by TUI rendering or the empty local replay snapshot. Release builds log and synthesize an output during normalization; debug builds panic before that repair occurs.

Suggested Fix

When creating ForkSnapshot::Interrupted, repair unmatched calls in the unfinished turn before appending the interruption boundary. Synthesize the same "aborted" outputs used by ensure_call_outputs_present, ideally covering:

  • FunctionCall
  • ToolSearchCall
  • CustomToolCall
  • LocalShellCall

Scope this to the unfinished turn suffix. Do not remove the debug panic globally, since that would hide malformed completed histories.

Regression Test

Add an app-server integration test that:

  1. Creates source history ending with an unmatched CustomToolCall.
  2. Calls thread/fork with ephemeral: true.
  3. Starts the first turn on the fork.
  4. Asserts it completes and the outbound model input has a matching aborted custom_tool_call_output.

A smaller core test should also assert that interrupted snapshot construction inserts the output before the interruption marker and TurnAborted.

The full backtrace follows.

› The application panicked (crashed).or compatibility
Message:  Custom tool call output is missing for call id: call_6D9SvwBze1C7efOZ28CBHt3o
Location: core/src/util.rs:9426754 · etraut/side-thread-nonblock… Side from main thread · Ctrl+C to return

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
                                ⋮ 7 frames hidden ⋮
   8: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h528552c70a667fb5
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/boxed.rs:2254
      2252 │ impl<Args: Tuple, F: Fn<Args> + ?Sized, A: Allocator> Fn<Args> for Box<F, A> {
      2253 │     extern "rust-call" fn call(&self, args: Args) -> Self::Output {
      2254 >         <F as Fn<Args>>::call(self, args)
      2255 │     }
      2256 │ }
   9: codex_tui::run_ratatui_app::{{closure}}::{{closure}}::hbc0fb0b0661005c3
      at /Users/felipe.coury/code/codex.etraut-side-thread-nonblocking-prepare/codex-rs/tui/src/lib.rs:1358
      1356 │     std::panic::set_hook(Box::new(move |info| {
      1357 │         tracing::error!("panic: {info}");
      1358 >         prev_hook(info);
      1359 │     }));
      1360 │     let mut initialized_terminal = tui::init()?;
  10: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h528552c70a667fb5
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/boxed.rs:2254
      2252 │ impl<Args: Tuple, F: Fn<Args> + ?Sized, A: Allocator> Fn<Args> for Box<F, A> {
      2253 │     extern "rust-call" fn call(&self, args: Args) -> Self::Output {
      2254 >         <F as Fn<Args>>::call(self, args)
      2255 │     }
      2256 │ }
  11: codex_tui::tui::set_panic_hook::{{closure}}::h474333abc1f6c411
      at /Users/felipe.coury/code/codex.etraut-side-thread-nonblocking-prepare/codex-rs/tui/src/tui.rs:494
       492 │     panic::set_hook(Box::new(move |panic_info| {
       493 │         let _ = restore_after_exit(); // ignore any errors as we are already failing
       494 >         hook(panic_info);
       495 │     }));
       496 │ }
  12: <alloc[1519c0c6343f008f]::boxed::Box<dyn for<'a, 'b> core[f63e075e1375d836]::ops::function::Fn<(&'a std[bb513d90a5cee88a]::panic::PanicHookInfo<'b>,), Output = ()> + core[f63e075e1375d836]::marker::Sync + core[f63e075e1375d836]::marker::Send> as core[f63e075e1375d836]::ops::function::Fn<(&std[bb513d90a5cee88a]::panic::PanicHookInfo,)>>::call<unknown>
      at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/alloc/src/boxed.rs:2254
  13: std[bb513d90a5cee88a]::panicking::panic_with_hook<unknown>
      at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/std/src/panicking.rs:833
  14: std[bb513d90a5cee88a]::panicking::panic_handler::{closure#0}<unknown>
      at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/std/src/panicking.rs:698
  15: std[bb513d90a5cee88a]::sys::backtrace::__rust_end_short_backtrace::<std[bb513d90a5cee88a]::panicking::panic_handler::{closure#0}, !><unknown>
      at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/std/src/sys/backtrace.rs:182
  16: __rustc[b7974e8690430dd9]::rust_begin_unwind<unknown>
      at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/std/src/panicking.rs:689
  17: core[f63e075e1375d836]::panicking::panic_fmt<unknown>
      at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/core/src/panicking.rs:80
  18: core::panicking::panic_display::h021b7bedb4c91eec
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/panicking.rs:259
       257 │ #[rustc_const_stable_indirect] // must follow stable const rules since it is exposed to stable
       258 │ pub const fn panic_display<T: fmt::Display>(x: &T) -> ! {
       259 >     panic_fmt(format_args!("{}", *x));
       260 │ }
       261 │
  19: codex_core::util::error_or_panic::h03e769288f85a54f
      at /Users/felipe.coury/code/codex.etraut-side-thread-nonblocking-prepare/codex-rs/core/src/util.rs:94
        92 │ pub(crate) fn error_or_panic(message: impl std::string::ToString) {
        93 │     if cfg!(debug_assertions) {
        94 >         panic!("{}", message.to_string());
        95 │     } else {
        96 │         error!("{}", message.to_string());
  20: codex_core::context_manager::normalize::ensure_call_outputs_present::h9babb8491a1dbf87
      at /Users/felipe.coury/code/codex.etraut-side-thread-nonblocking-prepare/codex-rs/core/src/context_manager/normalize.rs:75
        73 │
        74 │                 if !has_output {
        75 >                     error_or_panic(format!(
        76 │                         "Custom tool call output is missing for call id: {call_id}"
        77 │                     ));
  21: codex_core::context_manager::history::ContextManager::normalize_history::h4846616b7de191a0
      at /Users/felipe.coury/code/codex.etraut-side-thread-nonblocking-prepare/codex-rs/core/src/context_manager/history.rs:358
       356 │     fn normalize_history(&mut self, input_modalities: &[InputModality]) {
       357 │         // all function/tool calls must have a corresponding output
       358 >         normalize::ensure_call_outputs_present(&mut self.items);
       359 │
       360 │         // all outputs must have a corresponding function/tool call
  22: codex_core::context_manager::history::ContextManager::for_prompt::h67f5d96fa30315c3
      at /Users/felipe.coury/code/codex.etraut-side-thread-nonblocking-prepare/codex-rs/core/src/context_manager/history.rs:120
       118 │     /// outputs.
       119 │     pub(crate) fn for_prompt(mut self, input_modalities: &[InputModality]) -> Vec<ResponseItem> {
       120 >         self.normalize_history(input_modalities);
       121 │         self.items
       122 │     }
  23: codex_core::session::turn::run_turn::{{closure}}::h683505de83c07d09
      at /Users/felipe.coury/code/codex.etraut-side-thread-nonblocking-prepare/codex-rs/core/src/session/turn.rs:220
       218 │             sess.clone_history()
       219 │                 .await
       220 >                 .for_prompt(&turn_context.model_info.input_modalities)
       221 │         };

  24: <tracing::instrument::Instr
                                                                                                            e
       319 │         let (span, inner) = self.project
       372 │
       373 │                 let _guard = TaskIdGuard::enter(self.task_id);
       374 >                 future.poll(&mut cx)
       375 │             })
       376 │         };s::regular::RegularTask as codex_core::tasks::SessionTask>::run::{{closure}}::h0cda02  31: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut::hcecb8ef1dad2b6e9
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/loom/std/unsafe_cell.rs:16
        14 │     #[inline(always)]
        15 │     pub(crate) fn with_mut<R>(&self, f: impl FnOnce(*mut T) -> R) -> R {
        16 >         f(self.0.get())
        17 │     }       if !sess.input_queue.has_pending_input(&sess.active_turn).await {
        18 │ }               return last_agent_message;
  32: tokio::runtime::task::core::Core<T,S>::poll::h0dea634c39a070aa57dc6820194ec6
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/core.rs:363ture.rs:133
       361 │     pub(super) fn poll(&self, mut cx: Context<'_>) -> Poll<T::Output> {
       362 │         let res = {in<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
       363 >             self.stage.stage.with_mut(|ptr| {eref_mut(), cx)
       364 │                 // Safety: The caller ensures mutual exclusion to the field.
       365 │                 let future = match unsafe { &mut *ptr } {
  33: tokio::runtime::task::harness::poll_future::{{closure}}::h6185d36ac414f475k::{{closure}}::{{closure}}:      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/harness.rs:535lipe.coury/code/codex.etraut-side-thread-nonblocking-prepare/codex-rs/core/src/tasks/mod.r       533 │         }
       534 │         let guard = Guard { core };lation_token.child_token(),
       535 >         let res = guard.core.poll(cx);
       536 │         mem::forget(guard);
       537 │         res     let sess = session_ctx.clone_session();
  34: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::h52dd2fd522b75db6cing::instrument::Instrumented<T> as core::future::future::Future>::poll::h62e006092a7db26c
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/panic/unwind_safe.rs:274
       272 │     #[inline]span, inner) = self.project().span_and_inner_pin_mut();
       273 │     extern "rust-call" fn call_once(self, _args: ()) -> R {
       274 >         (self.0)()(cx)
       275 │     }
       276 │ }
  35: std::panicking::catch_unwind::do_call::h776b3d2f332b4e68l::h43a4fbec600dcd4e
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:581s:133
       579 │             let data = &mut (*data);
       580 │             let f = ManuallyDrop::take(&mut data.f);_>) -> Poll<Self::Output> {
       581 >             data.r = ManuallyDrop::new(f());deref_mut(), cx)
       582 │         }
       583 │     }
  36: ___rust_try<unknown>::core::Core<T,S>::poll::{{closure}}::hc1e87853be1ae8fc
      at <unknown source file>:<unknown line>src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/t  37: std::panicking::catch_unwind::h207e7bb1533b4859
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:544
       542 │     // See their safety preconditions for more information
       543 │     unsafe {
       544 >         return if intrinsics::catch_unwind(do_call::<F, R>, data_ptr, do_catch::<F, R>) == 0 {
       545 │             Ok(ManuallyDrop::into_inner(data.r))
       546 │         } else {
  38: std::panic::catch_unwind::h757adbd00075a6a0
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:359
       357 │ #[stable(feature = "catch_unwind", since = "1.9.0")]
       358 │ pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> {
       359 >     unsafe { panicking::catch_unwind(f) }
       360 │ }
       361 │
  39: tokio::runtime::task::harness::poll_future::h6f072474990ae0ed
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/harness.rs:523
       521 │ fn poll_future<T: Future, S: Schedule>(core: &Core<T, S>, cx: Context<'_>) -> Poll<()> {
       522 │     // Poll the future.
       523 >     let output = panic::catch_unwind(panic::AssertUnwindSafe(|| {
       524 │         struct Guard<'a, T: Future, S: Schedule> {
       525 │             core: &'a Core<T, S>,
  40: tokio::runtime::task::harness::Harness<T,S>::poll_inner::hdf252b053b663195
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/harness.rs:210
       208 │                 let waker_ref = waker_ref::<S>(&header_ptr);
       209 │                 let cx = Context::from_waker(&waker_ref);
       210 >                 let res = poll_future(self.core(), cx);
       211 │
       212 │                 if res == Poll::Ready(()) {
  41: tokio::runtime::task::harness::Harness<T,S>::poll::h132425129f2b3917
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/harness.rs:155
       153 │     pub(super) fn poll(self) {
       154 │         // We pass our ref-count to `poll_inner`.
       155 >         match self.poll_inner() {
       156 │             PollFuture::Notified => {
       157 │                 // The `poll_inner` call has given us two ref-counts back.
  42: tokio::runtime::task::raw::poll::hb484170aaddd2f6d
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/raw.rs:337
       335 │ unsafe fn poll<T: Future, S: Schedule>(ptr: NonNull<Header>) {
       336 │     let harness = Harness::<T, S>::from_raw(ptr);
       337 >     harness.poll();
       338 │ }
       339 │
  43: tokio::runtime::task::raw::RawTask::poll::h9bdb7dc3d0a791bf
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/raw.rs:267
       265 │     pub(crate) fn poll(self) {
       266 │         let vtable = self.header().vtable;
       267 >         unsafe { (vtable.poll)(self.ptr) }
       268 │     }
       269 │
  44: tokio::runtime::task::LocalNotified<S>::run::hf5774b1bd4b129bc
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/mod.rs:515
       513 │         let raw = self.task.raw;
       514 │         mem::forget(self);
       515 >         raw.poll();
       516 │     }
       517 │ }
  45: tokio::runtime::scheduler::multi_thread::worker::Context::run_task::{{closure}}::he7c34e5a1761a9f7
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/scheduler/multi_thread/worker.rs:643
       641 │                 .poll_start_callback(&task_meta);
       642 │
       643 >             task.run();
       644 │
       645 │             #[cfg(tokio_unstable)]
  46: tokio::task::coop::with_budget::h87a30ca3f83b5d9f
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/task/coop/mod.rs:167
       165 │     // The function is called regardless even if the budget is not successfully
       166 │     // set due to the thread-local being destroyed.
       167 >     f()
       168 │ }
       169 │
  47: tokio::task::coop::budget::h5e1c6a081a1b4672
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/task/coop/mod.rs:133
       131 │ #[inline(always)]
       132 │ pub(crate) fn budget<R>(f: impl FnOnce() -> R) -> R {
       133 >     with_budget(Budget::initial(), f)
       134 │ }
       135 │
  48: tokio::runtime::scheduler::multi_thread::worker::Context::run_task::he3e7685e7c521d8a
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/scheduler/multi_thread/worker.rs:634
       632 │
       633 │         // Run the task
       634 >         coop::budget(|| {
       635 │             // Unlike the poll time above, poll start callback is attached to the task id,
       636 │             // so it is tightly associated with the actual poll invocation.
  49: tokio::runtime::scheduler::multi_thread::worker::Context::run::h89887a97988c941b
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/scheduler/multi_thread/worker.rs:567
       565 │             // First, check work available to the current worker.
       566 │             if let Some(task) = core.next_task(&self.worker) {
       567 >                 core = self.run_task(task, core)?;
       568 │                 continue;
       569 │             }
  50: tokio::runtime::scheduler::multi_thread::worker::run::{{closure}}::{{closure}}::haa487c39c5439c89
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/scheduler/multi_thread/worker.rs:532
       530 │             // This should always be an error. It only returns a `Result` to support
       531 │             // using `?` to short circuit.
       532 >             assert!(cx.run(core).is_err());
       533 │
       534 │             // Check if there are any deferred tasks to notify. This can happen when
  51: tokio::runtime::context::scoped::Scoped<T>::set::h85f9f0b9825c217a
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/context/scoped.rs:40
        38 │         };
        39 │
        40 >         f()
        41 │     }
        42 │
  52: tokio::runtime::context::set_scheduler::{{closure}}::h1220ec4dde7ed6f7
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/context.rs:176
       174 │
       175 │     pub(super) fn set_scheduler<R>(v: &scheduler::Context, f: impl FnOnce() -> R) -> R {
       176 >         CONTEXT.with(|c| c.scheduler.set(v, f))
       177 │     }
       178 │
  53: std::thread::local::LocalKey<T>::try_with::hf4179eeabd0f2059
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:513
       511 │     {
       512 │         let thread_local = unsafe { (self.inner)(None).as_ref().ok_or(AccessError)? };
       513 >         Ok(f(thread_local))
       514 │     }
       515 │
  54: std::thread::local::LocalKey<T>::with::hb22103bf79dc8383
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:477
       475 │         F: FnOnce(&T) -> R,
       476 │     {
       477 >         match self.try_with(f) {
       478 │             Ok(r) => r,
       479 │             Err(err) => panic_access_error(err),
  55: tokio::runtime::context::set_scheduler::h4e9863f44da59640
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/context.rs:176
       174 │
       175 │     pub(super) fn set_scheduler<R>(v: &scheduler::Context, f: impl FnOnce() -> R) -> R {
       176 >         CONTEXT.with(|c| c.scheduler.set(v, f))
       177 │     }
       178 │
  56: tokio::runtime::scheduler::multi_thread::worker::run::{{closure}}::hf34063fad5a93b3c
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/scheduler/multi_thread/worker.rs:527
       525 │         });
       526 │
       527 >         context::set_scheduler(&cx, || {
       528 │             let cx = cx.expect_multi_thread();
       529 │
  57: tokio::runtime::context::runtime::enter_runtime::h3d022c6e9d700733
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/context/runtime.rs:65
        63 │
        64 │     if let Some(mut guard) = maybe_guard {
        65 >         return f(&mut guard.blocking);
        66 │     }
        67 │
  58: tokio::runtime::scheduler::multi_thread::worker::run::he79deb9f88cb336e
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/scheduler/multi_thread/worker.rs:519
       517 │     let handle = scheduler::Handle::MultiThread(worker.handle.clone());
       518 │
       519 >     crate::runtime::context::enter_runtime(&handle, true, |_| {
       520 │         // Set the worker context.
       521 │         let cx = scheduler::Context::MultiThread(Context {
  59: tokio::runtime::scheduler::multi_thread::worker::Launch::launch::{{closure}}::hf48ea04a03595d72
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/scheduler/multi_thread/worker.rs:485
       483 │     pub(crate) fn launch(mut self) {
       484 │         for worker in self.0.drain(..) {
       485 >             runtime::spawn_blocking(move || run(worker));
       486 │         }
       487 │     }
  60: <tokio::runtime::blocking::task::BlockingTask<T> as core::future::future::Future>::poll::h77cc953454e15c7f
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/blocking/task.rs:42
        40 │         crate::task::coop::stop();
        41 │
        42 >         Poll::Ready(func())
        43 │     }
        44 │ }
  61: tokio::runtime::task::core::Core<T,S>::poll::{{closure}}::h8744daa34c1d0aea
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/core.rs:374
       372 │
       373 │                 let _guard = TaskIdGuard::enter(self.task_id);
       374 >                 future.poll(&mut cx)
       375 │             })
       376 │         };
  62: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut::h1d94168cf3883628
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/loom/std/unsafe_cell.rs:16
        14 │     #[inline(always)]
        15 │     pub(crate) fn with_mut<R>(&self, f: impl FnOnce(*mut T) -> R) -> R {
        16 >         f(self.0.get())
        17 │     }
        18 │ }
  63: tokio::runtime::task::core::Core<T,S>::poll::hd24b6dbcddba6a2a
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/core.rs:363
       361 │     pub(super) fn poll(&self, mut cx: Context<'_>) -> Poll<T::Output> {
       362 │         let res = {
       363 >             self.stage.stage.with_mut(|ptr| {
       364 │                 // Safety: The caller ensures mutual exclusion to the field.
       365 │                 let future = match unsafe { &mut *ptr } {
  64: tokio::runtime::task::harness::poll_future::{{closure}}::heb4e7d301c04a234
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/harness.rs:535
       533 │         }
       534 │         let guard = Guard { core };
       535 >         let res = guard.core.poll(cx);
       536 │         mem::forget(guard);
       537 │         res
  65: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::he50f410b30abd6eb
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/panic/unwind_safe.rs:274
       272 │     #[inline]
       273 │     extern "rust-call" fn call_once(self, _args: ()) -> R {
       274 >         (self.0)()
       275 │     }
       276 │ }
  66: std::panicking::catch_unwind::do_call::hb1c0bc5b66416c4e
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:581
       579 │             let data = &mut (*data);
       580 │             let f = ManuallyDrop::take(&mut data.f);
       581 >             data.r = ManuallyDrop::new(f());
       582 │         }
       583 │     }
  67: ___rust_try<unknown>
      at <unknown source file>:<unknown line>
  68: std::panicking::catch_unwind::hcb5c696b108bbbe1
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:544
       542 │     // See their safety preconditions for more information
       543 │     unsafe {
       544 >         return if intrinsics::catch_unwind(do_call::<F, R>, data_ptr, do_catch::<F, R>) == 0 {
       545 │             Ok(ManuallyDrop::into_inner(data.r))
       546 │         } else {
  69: std::panic::catch_unwind::h3090a913659da4f8
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:359
       357 │ #[stable(feature = "catch_unwind", since = "1.9.0")]
       358 │ pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> {
       359 >     unsafe { panicking::catch_unwind(f) }
       360 │ }
       361 │
  70: tokio::runtime::task::harness::poll_future::hbf527ca6ea25afb8
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/harness.rs:523
       521 │ fn poll_future<T: Future, S: Schedule>(core: &Core<T, S>, cx: Context<'_>) -> Poll<()> {
       522 │     // Poll the future.
       523 >     let output = panic::catch_unwind(panic::AssertUnwindSafe(|| {
       524 │         struct Guard<'a, T: Future, S: Schedule> {
       525 │             core: &'a Core<T, S>,
  71: tokio::runtime::task::harness::Harness<T,S>::poll_inner::h6fa0c382287fab26
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/harness.rs:210
       208 │                 let waker_ref = waker_ref::<S>(&header_ptr);
       209 │                 let cx = Context::from_waker(&waker_ref);
       210 >                 let res = poll_future(self.core(), cx);
       211 │
       212 │                 if res == Poll::Ready(()) {
  72: tokio::runtime::task::harness::Harness<T,S>::poll::hd4279aec89cbb391
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/harness.rs:155
       153 │     pub(super) fn poll(self) {
       154 │         // We pass our ref-count to `poll_inner`.
       155 >         match self.poll_inner() {
       156 │             PollFuture::Notified => {
       157 │                 // The `poll_inner` call has given us two ref-counts back.
  73: tokio::runtime::task::raw::poll::hcdec45b98caffce5
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/raw.rs:337
       335 │ unsafe fn poll<T: Future, S: Schedule>(ptr: NonNull<Header>) {
       336 │     let harness = Harness::<T, S>::from_raw(ptr);
       337 >     harness.poll();
       338 │ }
       339 │
  74: tokio::runtime::task::raw::RawTask::poll::h9bdb7dc3d0a791bf
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/raw.rs:267
       265 │     pub(crate) fn poll(self) {
       266 │         let vtable = self.header().vtable;
       267 >         unsafe { (vtable.poll)(self.ptr) }
       268 │     }
       269 │
  75: tokio::runtime::task::UnownedTask<S>::run::h5f66dd536593e5c1
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/task/mod.rs:552
       550 │
       551 │         // Use the other ref-count to poll the task.
       552 >         raw.poll();
       553 │         // Decrement our extra ref-count
       554 │         drop(task);
  76: tokio::runtime::blocking::pool::Task::run::hd6f9607e9c30f3c5
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/blocking/pool.rs:161
       159 │
       160 │     fn run(self) {
       161 >         self.task.run();
       162 │     }
       163 │
  77: tokio::runtime::blocking::pool::Inner::run::h13dbfa8c94834b5d
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/blocking/pool.rs:516
       514 │                 self.metrics.dec_queue_depth();
       515 │                 drop(shared);
       516 >                 task.run();
       517 │
       518 │                 shared = self.shared.lock();
  78: tokio::runtime::blocking::pool::Spawner::spawn_thread::{{closure}}::hc958193d24aaf432
      at /Users/felipe.coury/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/blocking/pool.rs:474
       472 │             // Only the reference should be moved into the closure
       473 │             let _enter = rt.enter();
       474 >             rt.inner.blocking_spawner().inner.run(id);
       475 │             drop(shutdown_tx);
       476 │         })
  79: std::sys::backtrace::__rust_begin_short_backtrace::h8709ba4af90bcf37
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/backtrace.rs:166
       164 │     F: FnOnce() -> T,
       165 │ {
       166 >     let result = f();
       167 │
       168 │     // prevent this frame from being tail-call optimised away
  80: std::thread::lifecycle::spawn_unchecked::{{closure}}::{{closure}}::h88057f7e5f71c8aa
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/lifecycle.rs:91
        89 │         let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| {
        90 │             crate::sys::backtrace::__rust_begin_short_backtrace(|| hooks.run());
        91 >             crate::sys::backtrace::__rust_begin_short_backtrace(f)
        92 │         }));
        93 │         // SAFETY: `their_packet` as been built just above and moved by the
  81: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::hc5d40c25de30096b
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/panic/unwind_safe.rs:274
       272 │     #[inline]
       273 │     extern "rust-call" fn call_once(self, _args: ()) -> R {
       274 >         (self.0)()
       275 │     }
       276 │ }
  82: std::panicking::catch_unwind::do_call::hbf9d34346c7952ca
                                                                                                            •sWorkingck42s • esc to inte rupt)


       581 >             data.r = ManuallyDrop::new(f

       583 │     }                              … Side from main thread ·  ain finishe
  83: ___rust_try<unknown>
      at <unknown source file>:<unknown line>
  84: std::panicking::catch_unwind::ha50116b978b11051
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:544
       542 │     // See their safety preconditions for more information
       543 │     unsafe {
       544 >         return if intrinsics::catch_unwind(do_call::<F, R>, data_ptr, do_catch::<F, R>) == 0 {
       545 │             Ok(ManuallyDrop::into_inner(data.r))
       546 │         } else {
  85: std::panic::catch_unwind::h6de2fa65d3aff9f3
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:359
       357 │ #[stable(feature = "catch_unwind", since = "1.9.0")]
       358 │ pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> {
       359 >     unsafe { panicking::catch_unwind(f) }
       360 │ }
       361 │
  86: std::thread::lifecycle::spawn_unchecked::{{closure}}::h9e09ff3896f660b9
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/lifecycle.rs:89
        87 │     let rust_start = move || {
        88 │         let f = f.into_inner();
        89 >         let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| {
        90 │             crate::sys::backtrace::__rust_begin_short_backtrace(|| hooks.run());
        91 │             crate::sys::backtrace::__rust_begin_short_backtrace(f)
  87: core::ops::function::FnOnce::call_once{{vtable.shim}}::ha5024762ee3cfd82
      at /Users/felipe.coury/.rustup/toolchains/1.95.0-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250
       248 │     /// Performs the call operation.
       249 │     #[unstable(feature = "fn_traits", issue = "29625")]
       250 >     extern "rust-call" fn call_once(self, args: Args) -> Self::Output;
       251 │ }
       252 │
  88: <alloc[1519c0c6343f008f]::boxed::Box<dyn core[f63e075e1375d836]::ops::function::FnOnce<(), Output = ()> + core[f63e075e1375d836]::marker::Send> as core[f63e075e1375d836]::ops::function::FnOnce<()>>::call_once<unknown>
      at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/alloc/src/boxed.rs:2240
  89: <std[bb513d90a5cee88a]::sys::thread::unix::Thread>::new::thread_start<unknown>
      at /rustc/59807616e1fa2540724bfbac14d7976d7e4a3860/library/std/src/sys/thread/unix.rs:118
  90: __pthread_cond_wait<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
Warning: SpanTrace capture is Unsupported.
Ensure that you've setup a tracing-error ErrorLayer and the semver versions are compatible

}
}

pub(super) fn cancel_pending_side_start(&mut self) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Codex] Please retain cancellation/cleanup state for the forked child and suppress its ThreadStarted routing until preparation or cleanup has completed. Currently a canceled, half-prepared child can become selectable and later be removed while active.

}
self.side_threads.insert(child_thread_id, side_state);
// Prepared side threads switch through the selector's local replay path.
if let Err(err) = self

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Codex] Before selecting the prepared child, verify that the displayed thread is still parent_thread_id. If the parent changed or closed during preparation, treat this result as stale and clean up the child.

Comment thread codex-rs/tui/src/app.rs

// Keep rendering while /side prepares, but freeze input so drafts and shortcuts cannot
// race the pending transition. Esc remains available to cancel the preparation.
if self.pending_side_start.is_some() && !matches!(event, TuiEvent::Draw | TuiEvent::Resize)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Codex] Make cancellation state visible to App when the slash request is emitted, not only when StartSide is later dispatched. Otherwise an immediate Esc can miss cancellation and affect the parent turn before side preparation starts.

@etraut-openai

Copy link
Copy Markdown
Contributor Author

The crash is due to a known hole in the core's logic. It's documented with a TODO comment. The fix is complicated and doesn't belong in this PR.

The other code review comments are real problems that are easier to fix, but they belie complexity introduced with this PR.

I will need to rethink my approach. I'm going to close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants