Skip to content

fix(tui): prevent macos stderr from corrupting composer#24459

Merged
fcoury-oai merged 2 commits into
mainfrom
fcoury/macos-malloc-fix
May 25, 2026
Merged

fix(tui): prevent macos stderr from corrupting composer#24459
fcoury-oai merged 2 commits into
mainfrom
fcoury/macos-malloc-fix

Conversation

@fcoury-oai
Copy link
Copy Markdown
Contributor

Why

Fixes #17139.

On macOS, runtime diagnostics such as MallocStackLogging messages can be written directly to process stderr while the inline TUI owns the terminal. Those bytes paint into the same viewport as the composer without passing through the renderer or composer state, making diagnostic output appear to leak into the input area.

What Changed

  • Add a macOS terminal stderr guard while the inline TUI owns the viewport.
  • Restore stderr when Codex returns terminal ownership for external interactive programs, suspend/resume, panic handling, and normal shutdown.
  • Add an fd-level regression test that verifies output is suppressed only while terminal ownership is held and restored at each handoff boundary.

How to Test

  1. On macOS, launch the interactive TUI and leave the composer visible.
  2. Exercise the workflow that triggers an allocator/runtime stderr diagnostic during an active session, as reported in macOS: MallocStackLogging: can't turn off malloc stack logging because it was not enabled. appears when running Codex v0.118.0 #17139.
  3. Confirm the diagnostic no longer overwrites the active composer region.
  4. Suspend or exit the TUI and confirm subsequent terminal stderr output remains visible.

The platform diagnostic is environment-dependent, so the deterministic regression check is the new fd-lifecycle test in tui::terminal_stderr::tests::suppresses_stderr_only_while_terminal_is_owned.

Targeted validation:

  • just argument-comment-lint-from-source -p codex-tui passed.
  • just test -p codex-tui exercised and passed the new stderr-guard regression test. The full invocation currently fails in two unrelated guardian-policy tests, update_feature_flags_disabling_guardian_clears_review_policy_and_restores_default and update_feature_flags_disabling_guardian_clears_manual_review_policy_without_history, which reproduce when rerun in isolation.

@fcoury-oai fcoury-oai marked this pull request as ready for review May 25, 2026 16:22
Copy link
Copy Markdown
Contributor

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

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: 42970c9ccd

ℹ️ 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/tui/terminal_stderr.rs Outdated
Copy link
Copy Markdown
Collaborator

@etraut-openai etraut-openai left a comment

Choose a reason for hiding this comment

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

I like this approach.

Code looks good. Left one question about Mac-only choice.

I verified locally that the problem is fixed.

In light of this fix, I think we should consider reverting a71fc47 either partially or in its entirety. It was not the correct fix. If you agree, could you please do a follow-up PR?

use std::sync::MutexGuard;

#[cfg(target_os = "macos")]
static STDERR_STATE: Mutex<StderrState> = Mutex::new(StderrState {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is this MacOS only? Wouldn't we expect to see similar problems on Linux and maybe even Windows? Or are you just trying to keep the fix really targeted to reduce regression risk?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was working under the premise that I only really saw this issue in macOS, but your question made me think so I asked Codex to help me verify this assumption. And it seems like my hunch was correct, see below.

I think we should leave this as macOS only for the time being, unless you have a different opinion.

Item Relationship Reported platform Evidence
#17139 Directly fixed by #24459 macOS / Darwin arm64 Original report and detailed follow-up both explicitly identify macOS; symptom is repeated MallocStackLogging warning in the TUI.
#11555 Original issue addressed by earlier PR #16699 macOS Original report specifies macOS; one later commenter explicitly reports macOS 26.3.1. Other “same issue” comments do not state a platform.
#17081 Marked by Eric as duplicate of #11555 Darwin arm64 / Ghostty Reports leaked diagnostic output in the TUI; explicitly macOS.
#22661 Later duplicate report Darwin arm64 / Terminal.app Reports codex(pid) Malloc... in the composer/status area; explicitly macOS.
#23260 References #17139 as a secondary symptom Apple Silicon Primarily a subagent memory-growth report; mentions the malloc warning after degradation. Still Apple-platform evidence only.
#16699 Earlier attempted fix macOS-specific Title and body explicitly scope the diagnostic to macOS and add macOS-only environment scrubbing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also:

The warning family is specifically grounded in Apple malloc diagnostics. The local macOS malloc(3) documentation defines MallocStackLogging, MallocStackLoggingNoCompact, and MallocStackLoggingDirectory. That supports treating the reported diagnostic source as macOS-specific.

@fcoury-oai fcoury-oai closed this May 25, 2026
@fcoury-oai fcoury-oai reopened this May 25, 2026
@fcoury-oai
Copy link
Copy Markdown
Contributor Author

In light of this fix, I think we should consider reverting a71fc47 either partially or in its entirety. It was not the correct fix. If you agree, could you please do a follow-up PR?

Yes, I will open that PR shortly.

@fcoury-oai fcoury-oai enabled auto-merge (squash) May 25, 2026 19:52
@fcoury-oai fcoury-oai merged commit 599416d into main May 25, 2026
62 checks passed
@fcoury-oai fcoury-oai deleted the fcoury/macos-malloc-fix branch May 25, 2026 19:53
@github-actions github-actions Bot locked and limited conversation to collaborators May 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macOS: MallocStackLogging: can't turn off malloc stack logging because it was not enabled. appears when running Codex v0.118.0

2 participants