Skip to content

fix(io): catch ValueError in _SafeWriter for closed file handles - #2431

Closed
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:fix/2428-safewriter-valueerror
Closed

fix(io): catch ValueError in _SafeWriter for closed file handles#2431
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:fix/2428-safewriter-valueerror

Conversation

@Bartok9

@Bartok9 Bartok9 commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #2428_SafeWriter.isatty() raises ValueError on closed file during subagent ThreadPoolExecutor teardown.

Problem

When subagents run in ThreadPoolExecutor threads, the shared stdout handle can close between thread completion and KawaiiSpinner.stop() cleanup. Python raises ValueError (not OSError) for I/O operations on closed files:

ValueError: I/O operation on closed file

The _SafeWriter class (added in a8409a1) only catches OSError, missing this case.

Root Cause

Two commits that interact:

  1. a8409a1 (Mar 11) — Added _SafeWriter to catch OSError for broken pipes
  2. 6e2be33 (Mar 20) — Added isatty() check in KawaiiSpinner.stop()

The isatty() call during spinner cleanup can fail with ValueError when the underlying handle is already closed.

Fix

Add ValueError to exception handling in write(), flush(), and isatty() methods:

except (OSError, ValueError):

This is a standard Python pattern — ValueError: I/O operation on closed file is the expected exception for operations on closed file descriptors, distinct from OSError which covers broken pipes and device errors.

Testing

  • Verified the code paths handle both exception types
  • This is a straightforward exception expansion with no behavioral change for healthy streams

…sResearch#2428)

When subagents run in ThreadPoolExecutor threads, the shared stdout handle
can close between thread teardown and KawaiiSpinner cleanup. Python raises
ValueError (not OSError) for I/O operations on closed files:
  ValueError: I/O operation on closed file

The _SafeWriter class was only catching OSError, missing this case.

Changes:
- Add ValueError to exception handling in write(), flush(), and isatty()
- Update docstring to document the ThreadPoolExecutor teardown scenario

Fixes NousResearch#2428
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #2466 — cherry-picked with authorship preserved. Thanks!

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.

bug: _SafeWriter.isatty() raises ValueError on closed file (subagent ThreadPoolExecutor teardown)

2 participants