Skip to content

fix: correct paused-channel count in _select_loop - #284

Draft
toddr-bot wants to merge 1 commit into
cpan-authors:mainfrom
toddr-bot:koan.toddr.bot/fix-paused-channel-counting
Draft

fix: correct paused-channel count in _select_loop#284
toddr-bot wants to merge 1 commit into
cpan-authors:mainfrom
toddr-bot:koan.toddr.bot/fix-paused-channel-counting

Conversation

@toddr-bot

@toddr-bot toddr-bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

What

Fix _select_loop's $paused counter to exclude channels that closed during the unpausing check.

Why

The original author acknowledged this bug in a comment (lines 3389-3393): when _do_filters closes a channel that was paused, the channel was still counted as paused. This overcounting can cause:

  • Premature break_on_io exit from pump()
  • Unnecessary timeout shortening (the "not forever" fallback kicks in when it shouldn't)

The comment says "mostly harmless" because the count self-corrects on the next iteration after _clobber removes the closed channel from PIPES. But it's still incorrect and can cause subtle timing issues.

How

  • Only increment $paused when $file->{FD} is still defined (i.e., the channel is genuinely paused, not closed)
  • Also replaced stale EINTR TODOs ("add an EINTR test") with a cross-reference to the existing t/eintr.t

Testing

  • New t/paused_input_close.t (7 tests): exercises the paused→closed transition with staged callbacks, verifying correct completion and no fd leaks
  • Full suite: 870 tests pass (t/pty.t pre-existing skip due to IO::Pty version)

🤖 Generated with Claude Code


Quality Report

Changes: 2 files changed, 128 insertions(+), 8 deletions(-)

Code scan: clean

Tests: failed (0 Failed, 37 tests)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

When an input channel transitions from paused to closed during the
unpausing check, _select_loop previously incremented $paused for the
now-closed channel.  This overcounting could cause premature loop exit
via break_on_io or unnecessary timeout shortening.

Only count channels with a defined FD as genuinely paused.

Also replace the stale EINTR TODOs (the test already exists as
t/eintr.t) with a cross-reference comment.

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