From c2e0d6b800732847a591169164a474a93c9b0257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sun, 22 Feb 2026 00:53:15 -0700 Subject: [PATCH 01/10] fix: prevent filehandle reference leak in $_last_call_for _check() stored the file argument in $_last_call_for for -X _ caching. When the argument was a filehandle reference (not a string path), this prevented the filehandle from being garbage collected, keeping the underlying file descriptor open. This caused "spooky action-at-a-distance" bugs: e.g. a socketpair read hanging because a dup'd write-end filehandle was kept alive by the leaked reference, even after leaving scope. Fix: only cache string filenames in $_last_call_for, not references. Ref: https://github.com/cpanel/Test-MockFile/issues/179 Co-Authored-By: Claude Opus 4.6 --- lib/Overload/FileCheck.pm | 5 ++- t/fh-ref-leak.t | 66 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 t/fh-ref-leak.t diff --git a/lib/Overload/FileCheck.pm b/lib/Overload/FileCheck.pm index 51bf9d8..2d0927e 100644 --- a/lib/Overload/FileCheck.pm +++ b/lib/Overload/FileCheck.pm @@ -584,7 +584,10 @@ sub _check { $file = $_last_call_for if !defined $file && defined $_last_call_for && !defined $_current_mocks->{ $MAP_FC_OP{'stat'} }; my ( $out, @extra ) = $_current_mocks->{$optype}->($file); - $_last_call_for = $file; + # Only cache string filenames, not filehandle references. + # Storing a ref here prevents the filehandle from being garbage collected, + # causing resource leaks (e.g. sockets staying open). See GH #179. + $_last_call_for = ref($file) ? undef : $file; # FIXME return undef when not defined out diff --git a/t/fh-ref-leak.t b/t/fh-ref-leak.t new file mode 100644 index 0000000..98e5e58 --- /dev/null +++ b/t/fh-ref-leak.t @@ -0,0 +1,66 @@ +#!/usr/bin/perl + +# Test that file check operators do not retain references to filehandles +# passed as arguments. This prevents garbage collection of the filehandle, +# which can cause resource leaks (e.g. sockets staying open). +# +# See: https://github.com/cpanel/Test-MockFile/issues/179 + +use strict; +use warnings; + +use Test2::Bundle::Extended; +use Test2::Tools::Explain; + +use Scalar::Util qw(weaken); +use Overload::FileCheck -from_stat => \&my_stat, qw{:check}; + +sub my_stat { + my ( $stat_or_lstat, $f ) = @_; + return FALLBACK_TO_REAL_OP(); +} + +# Test that filehandle references are not retained by $_last_call_for +{ + 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" ); + + # Trigger a file check on the filehandle — this used to store $fh + # in $_last_call_for, preventing garbage collection. + no warnings; + -f $fh; + } + + ok( !defined $weak_ref, "filehandle is garbage collected after -f check (no ref leak)" ); +} + +# Test with -S (the operator from the original bug report) +{ + 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 check (no ref leak)" ); +} + +# Test that string filenames still work for _ caching (no regression) +{ + no warnings; + ok( -f $0, "-f \$0 works" ); + ok( -e _, "-e _ works after -f on string filename" ); +} + +done_testing; From f94a776e3246ead5244a487c3aa61c1c97765a5e Mon Sep 17 00:00:00 2001 From: "Nicolas R." Date: Sun, 22 Feb 2026 09:56:29 -0700 Subject: [PATCH 02/10] Remove duplicate strict/warnings --- examples/synopsis.pl | 3 --- 1 file changed, 3 deletions(-) diff --git a/examples/synopsis.pl b/examples/synopsis.pl index 77f0f84..a014720 100644 --- a/examples/synopsis.pl +++ b/examples/synopsis.pl @@ -3,9 +3,6 @@ use strict; use warnings; -use strict; -use warnings; - use Test::More; use Overload::FileCheck q{:all}; From fe4051a17168f23f1f9a2b3b4dc7efd21b78b60d Mon Sep 17 00:00:00 2001 From: "Nicolas R." Date: Sun, 22 Feb 2026 09:56:39 -0700 Subject: [PATCH 03/10] Makefile.PL auto-updated by dist.ini --- Makefile.PL | 2 +- README.md | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/Makefile.PL b/Makefile.PL index c8afb4d..85dd29f 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -1,4 +1,4 @@ -# This file was automatically generated by Dist::Zilla::Plugin::MakeMaker v6.031. +# This file was automatically generated by Dist::Zilla::Plugin::MakeMaker v6.033. use strict; use warnings; diff --git a/README.md b/README.md index 376e62a..586e37a 100644 --- a/README.md +++ b/README.md @@ -20,9 +20,6 @@ By using mock\_all\_file\_checks you can set a hook function to reply any -X che use strict; use warnings; -use strict; -use warnings; - use Test::More; use Overload::FileCheck q{:all}; From 620928ff01a057c3d913993b63962f0d71d2a7b6 Mon Sep 17 00:00:00 2001 From: "Nicolas R." Date: Sun, 22 Feb 2026 09:57:09 -0700 Subject: [PATCH 04/10] ignore local/ dir --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index d2e49e2..b22983e 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ Makefile.old MANIFEST.bak Overload-FileCheck-* .build +local/ From 48564319acefa98bf621119b03fc55ee8f347f19 Mon Sep 17 00:00:00 2001 From: "Nicolas R." Date: Sun, 22 Feb 2026 09:59:56 -0700 Subject: [PATCH 05/10] Bump copyright year --- README.md | 2 +- dist.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 586e37a..6ce7056 100644 --- a/README.md +++ b/README.md @@ -752,7 +752,7 @@ Nicolas R # COPYRIGHT AND LICENSE -This software is copyright (c) 2022 by cPanel, L.L.C. +This software is copyright (c) 2026 by cPanel, L.L.C. This is free software; you can redistribute it and/or modify it under the same terms as the Perl 5 programming language system itself. diff --git a/dist.ini b/dist.ini index 57380a6..7ac7121 100644 --- a/dist.ini +++ b/dist.ini @@ -2,7 +2,7 @@ name = Overload-FileCheck author = Nicolas R license = Perl_5 copyright_holder = cPanel, L.L.C. -copyright_year = 2022 +copyright_year = 2026 ;[PPPort] From 737a2822f8d74141f79d2740f93d82395710ffae Mon Sep 17 00:00:00 2001 From: "Nicolas R." Date: Sun, 22 Feb 2026 10:01:49 -0700 Subject: [PATCH 06/10] Prep changes for release --- Changes | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Changes b/Changes index 5e0ecf4..5485d1a 100644 --- a/Changes +++ b/Changes @@ -2,6 +2,9 @@ Revision history for Overload-FileCheck {{$NEXT}} +- fix: prevent filehandle reference leak in $_last_call_for +- fix: spelling typos + 0.013 2022-02-23 08:36:12-07:00 America/Denver - Fix a PL_statcache bug when checking: -l $f || -e _ From 5544cec9be1dc08112cf6fee36f3dc53ee8a48a0 Mon Sep 17 00:00:00 2001 From: "Nicolas R." Date: Sun, 22 Feb 2026 10:09:17 -0700 Subject: [PATCH 07/10] Add CLAUDE.md and exclude dev files from distribution Add CLAUDE.md with project guidance for Claude Code sessions. Exclude CLAUDE.md and local/ directory from Dist::Zilla GatherDir so they don't end up in the CPAN tarball. --- CLAUDE.md | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ dist.ini | 2 ++ 2 files changed, 54 insertions(+) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..7a81b91 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,52 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project Overview + +Overload::FileCheck is a Perl XS module that hooks into Perl's OP dispatch mechanism to mock file check operators (-e, -f, -d, -s, etc.) and stat/lstat at the interpreter level. Designed for unit testing scenarios where you need to simulate filesystem conditions without real files. + +## Build & Test Commands + +```bash +# Build (compiles XS/C code) +perl Makefile.PL && make + +# Run full test suite +make test + +# Run a single test +prove -lv t/02_basic-mock.t + +# Author testing via Dist::Zilla +dzil test + +# Build a release +dzil build +``` + +Makefile.PL is auto-generated from dist.ini — edit dist.ini for build configuration, not Makefile.PL directly. + +## Architecture + +### Two-layer design + +**Perl layer** (`lib/Overload/FileCheck.pm`): Public API — `mock_file_check`, `mock_stat`, `mock_lstat`, `mock_all_file_checks`, `mock_all_from_stat`, and their `unmock_*` counterparts. Manages the mapping from operator names (e.g., '-e') to Perl OP types (e.g., `OP_FTIS`). Provides export groups `:check` (return value constants), `:stat` (stat index constants and helpers like `stat_as_file()`), and `:all`. + +**XS layer** (`FileCheck.xs` + `FileCheck.h`): Replaces Perl's default `pp_*` OP handlers with custom ones that call back into Perl. Three handler types: `pp_overload_ft_yes_no` (boolean ops like -e, -f), `pp_overload_ft_int`/`pp_overload_ft_nv` (numeric ops like -s, -M), and `pp_overload_stat` (stat/lstat). `FileCheck.h` contains compatibility macros for Perl 5.14 vs 5.15+ internal API differences. + +### Return value protocol + +Mock callbacks return: `CHECK_IS_TRUE` (1), `CHECK_IS_FALSE` (0), or `FALLBACK_TO_REAL_OP` (-1) for boolean ops. Numeric ops (-s, -M, -C, -A) return the actual value. Stat mocks return an arrayref of 13 elements (or empty arrayref for "file not found"). + +### Test structure + +Tests in `t/` use Test2 framework. Many test files for individual operators (test-e.t, test-f.t, etc.) are symlinks to template files (`test-true-false.t` for boolean ops, `test-integer.t` for numeric ops) — the test determines which operator to exercise based on its own filename. + +## Key Conventions + +- Minimum Perl version: **5.010** (enforced in dist.ini and CI) +- CI tests across Perl 5.10 through latest dev release +- Operators accepted with or without dash: `'-e'` and `'e'` are equivalent +- Code style: PerlTidy with 2-space indent (see `tidyall.ini`), PerlCritic severity 3 +- Distribution managed with **Dist::Zilla** (`dist.ini`); `[@Git]` plugin handles version tagging and push diff --git a/dist.ini b/dist.ini index 7ac7121..3354452 100644 --- a/dist.ini +++ b/dist.ini @@ -80,7 +80,9 @@ run = git status --porcelain | grep 'M Makefile.PL' && git commit -m 'Makefile.P [GatherDir] exclude_filename = Makefile.PL +exclude_filename = CLAUDE.md ;exclude_filename = ppport.h +exclude_match = ^local/ ; -- static meta-information [MetaResources] From 48e1de3f82205b9aa130424245bb12074b0cf4e9 Mon Sep 17 00:00:00 2001 From: "Nicolas R." Date: Sun, 22 Feb 2026 10:13:53 -0700 Subject: [PATCH 08/10] Update actions/checkout and CI --- .github/workflows/author-testing.yml | 20 +++++--------------- .github/workflows/testsuite.yml | 6 +++--- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/.github/workflows/author-testing.yml b/.github/workflows/author-testing.yml index ef18681..a826301 100644 --- a/.github/workflows/author-testing.yml +++ b/.github/workflows/author-testing.yml @@ -3,7 +3,7 @@ name: author-testing on: push: branches: - - "*" + - "main" tags-ignore: - "*" pull_request: @@ -17,26 +17,16 @@ jobs: AUTHOR_TESTING: 1 AUTOMATED_TESTING: 1 RELEASE_TESTING: 1 - PERL_CARTON_PATH: $GITHUB_WORKSPACE/local - - strategy: - fail-fast: false - matrix: - perl-version: - - "5.30" container: - image: perldocker/perl-tester:${{ matrix.perl-version }} + image: perldocker/perl-tester:5.42 steps: - - uses: actions/checkout@v1 - - name: perl -V - run: perl -V + - uses: actions/checkout@v6 + - run: perl -V - name: Install Author Dependencies run: dzil authordeps | cpm install -g --show-build-log-on-failure - - name: Install Dependencies - run: dzil listdeps | cpanm - # cannot use cpm due to https://github.com/skaji/cpm/issues/161 - #run: dzil listdeps | cpm install -g --show-build-log-on-failure - + run: dzil listdeps | cpm install -g --show-build-log-on-failure - - name: dzil test run: dzil test diff --git a/.github/workflows/testsuite.yml b/.github/workflows/testsuite.yml index 9b4e054..8af84cc 100644 --- a/.github/workflows/testsuite.yml +++ b/.github/workflows/testsuite.yml @@ -3,7 +3,7 @@ name: linux on: push: branches: - - "*" + - "main" tags-ignore: - "*" pull_request: @@ -21,7 +21,7 @@ jobs: PERL_CARTON_PATH: $GITHUB_WORKSPACE/local steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - run: perl -V - name: Install Dependencies uses: perl-actions/install-with-cpm@v1 @@ -66,7 +66,7 @@ jobs: container: perldocker/perl-tester:${{ matrix.perl-version }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - run: perl -V - name: Install Dependencies run: cpm install -g --show-build-log-on-failure From a94ca4750ec548b2cc8d36b538d01309bb827037 Mon Sep 17 00:00:00 2001 From: "Nicolas R." Date: Sun, 22 Feb 2026 10:19:41 -0700 Subject: [PATCH 09/10] v0.014 --- Changes | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changes b/Changes index 5485d1a..1dd1beb 100644 --- a/Changes +++ b/Changes @@ -2,6 +2,8 @@ Revision history for Overload-FileCheck {{$NEXT}} +0.014 2026-02-22 10:19:05-07:00 America/Denver + - fix: prevent filehandle reference leak in $_last_call_for - fix: spelling typos From deb1bdb0c9c7368988b72c444fe9b5f54d7549b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sat, 7 Mar 2026 14:57:44 -0700 Subject: [PATCH 10/10] fix: balance SvREFCNT_inc with SvREFCNT_dec in pp_overload_ft_nv _overload_ft_ops_sv() increments the refcount of the returned SV to keep it alive past FREETMPS, but pp_overload_ft_nv never decremented it, leaking one SV per mocked -M/-C/-A call. Add SvREFCNT_dec(status) before each return path in pp_overload_ft_nv. Co-Authored-By: Claude Opus 4.6 --- FileCheck.xs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/FileCheck.xs b/FileCheck.xs index c8c20a6..165a672 100644 --- a/FileCheck.xs +++ b/FileCheck.xs @@ -310,11 +310,15 @@ PP(pp_overload_ft_nv) { status = _overload_ft_ops_sv(); - if ( SvIOK(status) && SvIV(status) == -1 ) + if ( SvIOK(status) && SvIV(status) == -1 ) { + SvREFCNT_dec(status); return CALL_REAL_OP(); + } - if ( SvNOK(status) && SvNV(status) == -1 ) + if ( SvNOK(status) && SvNV(status) == -1 ) { + SvREFCNT_dec(status); return CALL_REAL_OP(); + } { dTARGET; @@ -325,6 +329,7 @@ PP(pp_overload_ft_nv) { else if ( SvIOK(status) ) sv_setiv(TARG, (IV) SvIV(status) ); + SvREFCNT_dec(status); FT_RETURN_TARG; } }