From b3e29210c06ff2ae1ab4adc0aacabdb7232d5cda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sun, 22 Feb 2026 00:53:19 -0700 Subject: [PATCH 1/4] test: add regression tests for filehandle reference leak (GH #179) Tests that file check operators (-S, -f, etc.) on real filehandles do not retain references that prevent garbage collection. Includes the exact socketpair scenario from the bug report where a dup'd write handle kept alive by a leaked ref caused reads to hang. Root cause is in Overload::FileCheck's $_last_call_for variable. Ref: https://github.com/cpanel/Test-MockFile/issues/179 Co-Authored-By: Claude Opus 4.6 --- t/fh-ref-leak.t | 111 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 t/fh-ref-leak.t diff --git a/t/fh-ref-leak.t b/t/fh-ref-leak.t new file mode 100644 index 0000000..9b69891 --- /dev/null +++ b/t/fh-ref-leak.t @@ -0,0 +1,111 @@ +#!/usr/bin/perl -w + +# Test for GitHub issue #179: "Spooky action-at-a-distance" +# +# File check operators (-S, -f, etc.) on real (unmocked) filehandles should +# not retain references that prevent garbage collection. A leaked reference +# to a socket filehandle can keep the fd open, causing reads on the other +# end of a socketpair to hang waiting for EOF. +# +# Root cause: $_last_call_for in Overload::FileCheck stored filehandle refs. +# Fix: Only cache string filenames, not refs. + +use strict; +use warnings; + +use Test2::Bundle::Extended; +use Test2::Tools::Explain; + +use Scalar::Util qw(weaken); +use Socket; + +use Test::MockFile qw< nostrict >; + +# Probe: check if Overload::FileCheck has the ref leak fix. +# Without it (O::FC PR #25), $_last_call_for retains fh refs. +my $ofc_has_fix; +{ + my $probe; + { + open my $fh, '<', '/dev/null' or die "Cannot open /dev/null: $!"; + $probe = $fh; + Scalar::Util::weaken($probe); + no warnings; + -f $fh; + } + $ofc_has_fix = !defined $probe; +} + +# Test 1: Filehandle passed to -f is not retained +SKIP: { + skip "Overload::FileCheck does not have ref leak fix (PR #25)", 1 unless $ofc_has_fix; + my $weak_ref; + + { + open my $fh, '<', '/dev/null' or die "Cannot open /dev/null: $!"; + $weak_ref = $fh; + weaken($weak_ref); + + ok( defined $weak_ref, "weak ref is defined before scope exit" ); + + no warnings; + -f $fh; + } + + ok( !defined $weak_ref, "filehandle is garbage collected after -f (GH #179)" ); +} + +# Test 2: Socket filehandle passed to -S is not retained +SKIP: { + skip "Overload::FileCheck does not have ref leak fix (PR #25)", 1 unless $ofc_has_fix; + my $weak_ref; + + { + open my $fh, '<', '/dev/null' or die "Cannot open /dev/null: $!"; + $weak_ref = $fh; + weaken($weak_ref); + + no warnings; + -S $fh; + } + + ok( !defined $weak_ref, "filehandle is garbage collected after -S (GH #179)" ); +} + +# Test 3: The exact scenario from GH #179 — socketpair with dup'd fd +# This would hang without the fix because the dup'd write handle stays open. +SKIP: { + skip "Overload::FileCheck does not have ref leak fix (PR #25)", 1 unless $ofc_has_fix; + skip "socketpair not available", 1 unless eval { socketpair my $a, my $b, AF_UNIX, SOCK_STREAM, 0; 1 }; + + my $pid = fork(); + if ( !defined $pid ) { + skip "fork not available", 1; + } + + if ( $pid == 0 ) { + # Child: reproduce the bug scenario with a timeout + $SIG{ALRM} = sub { exit 1 }; # exit 1 = hung (bug present) + alarm(5); + + socketpair my $r, my $w, AF_UNIX, SOCK_STREAM, 0 + or exit 2; + + my $fd = fileno $w; + do { + open my $w2, "<&=", $fd; + -S $w2; + }; + + close $w; + my $line = <$r>; # Should get EOF immediately if $w2 was freed + exit 0; # exit 0 = success (no hang) + } + + waitpid $pid, 0; + my $exit = $? >> 8; + + is( $exit, 0, "socketpair read does not hang after -S on dup'd filehandle (GH #179)" ); +} + +done_testing; From f275b40bf972a4648ddea2fc439261e422f97802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Wed, 25 Feb 2026 18:38:57 -0700 Subject: [PATCH 2/4] rebase: apply review feedback on #204 --- .claude/worktrees/agent-a12e377c | 1 + t/fh-ref-leak.t | 24 +++--------------------- 2 files changed, 4 insertions(+), 21 deletions(-) create mode 160000 .claude/worktrees/agent-a12e377c diff --git a/.claude/worktrees/agent-a12e377c b/.claude/worktrees/agent-a12e377c new file mode 160000 index 0000000..a024519 --- /dev/null +++ b/.claude/worktrees/agent-a12e377c @@ -0,0 +1 @@ +Subproject commit a0245197aaf37c9abdcdb300c114108155ce3850 diff --git a/t/fh-ref-leak.t b/t/fh-ref-leak.t index 9b69891..d7d461c 100644 --- a/t/fh-ref-leak.t +++ b/t/fh-ref-leak.t @@ -8,7 +8,7 @@ # end of a socketpair to hang waiting for EOF. # # Root cause: $_last_call_for in Overload::FileCheck stored filehandle refs. -# Fix: Only cache string filenames, not refs. +# Fix: Only cache string filenames, not refs (Overload::FileCheck PR #25). use strict; use warnings; @@ -21,24 +21,8 @@ use Socket; use Test::MockFile qw< nostrict >; -# Probe: check if Overload::FileCheck has the ref leak fix. -# Without it (O::FC PR #25), $_last_call_for retains fh refs. -my $ofc_has_fix; -{ - my $probe; - { - open my $fh, '<', '/dev/null' or die "Cannot open /dev/null: $!"; - $probe = $fh; - Scalar::Util::weaken($probe); - no warnings; - -f $fh; - } - $ofc_has_fix = !defined $probe; -} - # Test 1: Filehandle passed to -f is not retained -SKIP: { - skip "Overload::FileCheck does not have ref leak fix (PR #25)", 1 unless $ofc_has_fix; +{ my $weak_ref; { @@ -56,8 +40,7 @@ SKIP: { } # Test 2: Socket filehandle passed to -S is not retained -SKIP: { - skip "Overload::FileCheck does not have ref leak fix (PR #25)", 1 unless $ofc_has_fix; +{ my $weak_ref; { @@ -75,7 +58,6 @@ SKIP: { # Test 3: The exact scenario from GH #179 — socketpair with dup'd fd # This would hang without the fix because the dup'd write handle stays open. SKIP: { - skip "Overload::FileCheck does not have ref leak fix (PR #25)", 1 unless $ofc_has_fix; skip "socketpair not available", 1 unless eval { socketpair my $a, my $b, AF_UNIX, SOCK_STREAM, 0; 1 }; my $pid = fork(); From e70f53a16d15d0513dfe2453c0175209197628f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Wed, 25 Feb 2026 18:44:14 -0700 Subject: [PATCH 3/4] rebase: apply review feedback on #204 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove .claude/worktrees/ submodule entry from tracking - Add .claude/ to .gitignore - Remove SKIP wrapper from socketpair test — Overload::FileCheck fix (cpanel/Overload-FileCheck#25) is now released Co-Authored-By: Claude Opus 4.6 --- .claude/worktrees/agent-a12e377c | 1 - .gitignore | 1 + t/fh-ref-leak.t | 13 +++++-------- 3 files changed, 6 insertions(+), 9 deletions(-) delete mode 160000 .claude/worktrees/agent-a12e377c diff --git a/.claude/worktrees/agent-a12e377c b/.claude/worktrees/agent-a12e377c deleted file mode 160000 index a024519..0000000 --- a/.claude/worktrees/agent-a12e377c +++ /dev/null @@ -1 +0,0 @@ -Subproject commit a0245197aaf37c9abdcdb300c114108155ce3850 diff --git a/.gitignore b/.gitignore index 07d5e7f..777b362 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ pm_to_blib Test-MockFile-* Test-MockFile-*.tar.gz .DS_Store +.claude/ # VIM - https://github.com/github/gitignore/blob/main/Global/Vim.gitignore # Swap diff --git a/t/fh-ref-leak.t b/t/fh-ref-leak.t index d7d461c..a32310c 100644 --- a/t/fh-ref-leak.t +++ b/t/fh-ref-leak.t @@ -57,22 +57,18 @@ use Test::MockFile qw< nostrict >; # Test 3: The exact scenario from GH #179 — socketpair with dup'd fd # This would hang without the fix because the dup'd write handle stays open. -SKIP: { - skip "socketpair not available", 1 unless eval { socketpair my $a, my $b, AF_UNIX, SOCK_STREAM, 0; 1 }; +{ + socketpair my $r, my $w, AF_UNIX, SOCK_STREAM, 0 + or die "socketpair: $!"; my $pid = fork(); - if ( !defined $pid ) { - skip "fork not available", 1; - } + die "fork: $!" unless defined $pid; if ( $pid == 0 ) { # Child: reproduce the bug scenario with a timeout $SIG{ALRM} = sub { exit 1 }; # exit 1 = hung (bug present) alarm(5); - socketpair my $r, my $w, AF_UNIX, SOCK_STREAM, 0 - or exit 2; - my $fd = fileno $w; do { open my $w2, "<&=", $fd; @@ -84,6 +80,7 @@ SKIP: { exit 0; # exit 0 = success (no hang) } + close $w; waitpid $pid, 0; my $exit = $? >> 8; From 36ea1cc8611fe516a87394651d5b955846ee8721 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Wed, 25 Feb 2026 20:50:58 -0700 Subject: [PATCH 4/4] rebase: apply review feedback on #204 --- Makefile.PL | 2 +- cpanfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.PL b/Makefile.PL index e6935d5..e4433c0 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -26,7 +26,7 @@ WriteMakefile( 'Test::MockModule' => 0, }, PREREQ_PM => { - 'Overload::FileCheck' => '0.013', + 'Overload::FileCheck' => '0.014', 'Text::Glob' => 0, }, dist => { COMPRESS => 'gzip -9f', SUFFIX => 'gz', }, diff --git a/cpanfile b/cpanfile index 5a9ec29..4e19f2a 100644 --- a/cpanfile +++ b/cpanfile @@ -13,7 +13,7 @@ on 'test' => sub { requires 'Test2::Tools::Explain' => 0; requires 'Test2::Plugin::NoWarnings' => 0; requires 'File::Slurper' => 0; - requires 'Overload::FileCheck' => '0.013'; + requires 'Overload::FileCheck' => '0.014'; requires 'Test::Pod::Coverage' => 0; requires 'Test::Pod' => 0; requires 'Test2::API' => 0;