Skip to content

fix: wrap pipe and IO cleanup in eval to prevent cascading failures - #280

Draft
toddr-bot wants to merge 1 commit into
cpan-authors:mainfrom
toddr-bot:koan.toddr.bot/fix-cleanup-error-resilience
Draft

fix: wrap pipe and IO cleanup in eval to prevent cascading failures#280
toddr-bot wants to merge 1 commit into
cpan-authors:mainfrom
toddr-bot:koan.toddr.bot/fix-cleanup-error-resilience

Conversation

@toddr-bot

Copy link
Copy Markdown
Contributor

What

Protect the pipe clobber loop and IO _cleanup() calls in _cleanup() with eval blocks.

Why

In _cleanup(), the PTY close phase was already protected by eval blocks, but two subsequent phases were not:

  1. The pipe clobber loop ($self->_clobber(...))
  2. The per-kid IO _cleanup() calls

If either throws (e.g., close fails on a corrupted fd), subsequent cleanup steps — child reaping, filter teardown, DEBUG_FD close — are skipped entirely, leaking resources.

How

Wrapped both unprotected phases in eval with carp on error, matching the existing PTY cleanup pattern. All cleanup phases now run even if one fails.

Testing

  • Full test suite passes (863 tests, pty.t skipped for IO::Pty < 1.25)
  • Defensive fix — the failure mode requires a close() to throw during cleanup, which is rare but not impossible (corrupted fd, NFS stale handle, etc.)

🤖 Generated with Claude Code

In _cleanup(), the PTY close phase was already protected by eval blocks,
but the pipe clobber loop and IO _cleanup() calls were not. If either
throws (e.g., a close fails on a corrupted fd), subsequent cleanup
steps — child reaping, filter teardown, DEBUG_FD close — would be
skipped entirely, leaking resources.

Wrap both unprotected phases in eval with carp on error, matching the
existing PTY cleanup pattern. This ensures all cleanup phases run even
if one fails.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

1 participant