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 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/ 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/Changes b/Changes index 5e0ecf4..1dd1beb 100644 --- a/Changes +++ b/Changes @@ -2,6 +2,11 @@ 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 + 0.013 2022-02-23 08:36:12-07:00 America/Denver - Fix a PL_statcache bug when checking: -l $f || -e _ 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..6ce7056 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}; @@ -755,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..3354452 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] @@ -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] 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}; diff --git a/lib/Overload/FileCheck.pm b/lib/Overload/FileCheck.pm index 51bf9d8..6902c40 100644 --- a/lib/Overload/FileCheck.pm +++ b/lib/Overload/FileCheck.pm @@ -62,7 +62,7 @@ my @STAT_HELPERS = qw{ stat_as_directory stat_as_file stat_as_symlink our @EXPORT_OK = ( qw{ mock_all_from_stat - mock_all_file_checks mock_file_check mock_stat + mock_all_file_checks mock_file_check mock_file_check_guard mock_stat unmock_file_check unmock_all_file_checks unmock_stat }, @CHECK_STATUS, @@ -235,6 +235,16 @@ sub mock_file_check { return 1; } +sub mock_file_check_guard { + my ( $check, $sub ) = @_; + + mock_file_check( $check, $sub ); + + ( my $normalized = $check ) =~ s{^-+}{}; + + return Overload::FileCheck::Guard->new($normalized); +} + sub unmock_file_check { my (@checks) = @_; @@ -584,7 +594,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 @@ -767,6 +780,36 @@ sub _stat_for { return \@stat; } +###################################################### +### Scope guard for automatic mock cleanup +###################################################### + +package Overload::FileCheck::Guard; + +sub new { + my ( $class, @checks ) = @_; + + return bless { checks => \@checks, active => 1 }, $class; +} + +sub cancel { + my ($self) = @_; + + $self->{active} = 0; + return; +} + +sub DESTROY { + my ($self) = @_; + + return unless $self->{active}; + $self->{active} = 0; + + local $@; + eval { Overload::FileCheck::unmock_file_check( @{ $self->{checks} } ) }; + return; +} + 1; =pod @@ -1062,6 +1105,21 @@ Otherwise returns 1 on success. # in that sample all '-e' checks will always return true... mock_file_check( '-e' => sub { 1 } ) +=head2 mock_file_check_guard( $check, CODE ) + +Like C, but returns a guard object instead of C<1>. +When the guard goes out of scope (or is otherwise destroyed), the mock is +automatically removed via C. This improves test isolation +by guaranteeing cleanup even if the test dies. + + { + my $guard = mock_file_check_guard( '-e' => sub { CHECK_IS_TRUE } ); + ok( -e "/fake/file", "mocked" ); + } + # -e is automatically unmocked here + +Call C<< $guard->cancel >> to prevent the automatic unmock. + =head2 unmock_file_check( $check, [@extra_checks] ) Disable the effect of one or more specific mock. 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; diff --git a/t/guard.t b/t/guard.t new file mode 100644 index 0000000..e3275eb --- /dev/null +++ b/t/guard.t @@ -0,0 +1,65 @@ +use strict; +use warnings; + +use Test2::Bundle::Extended; +use Test2::Tools::Explain; + +use Overload::FileCheck qw( + mock_file_check_guard mock_file_check unmock_file_check + CHECK_IS_TRUE CHECK_IS_FALSE FALLBACK_TO_REAL_OP +); + +my $fake = "/guard/test/file"; + +# --- basic guard: mock is active inside scope, removed after --- +{ + my $guard = mock_file_check_guard( '-e' => sub { CHECK_IS_TRUE } ); + isa_ok( $guard, 'Overload::FileCheck::Guard' ); + ok( -e $fake, "mocked -e returns true inside guard scope" ); +} +ok( !-e $fake, "-e falls back to real op after guard is destroyed" ); + +# --- guard with cancel: mock persists after scope --- +{ + my $guard = mock_file_check_guard( '-f' => sub { CHECK_IS_TRUE } ); + ok( -f $fake, "mocked -f returns true" ); + $guard->cancel; +} +# mock still active because we cancelled the guard +ok( -f $fake, "-f still mocked after cancelled guard" ); +unmock_file_check('-f'); # manual cleanup +ok( !-f $fake, "-f unmocked manually" ); + +# --- guard handles double-destroy gracefully --- +{ + my $guard = mock_file_check_guard( '-d' => sub { CHECK_IS_TRUE } ); + ok( -d $fake, "mocked -d" ); + # explicitly destroy, then let scope destroy again + $guard->DESTROY; + ok( !-d $fake, "-d unmocked after explicit DESTROY" ); +} +# second DESTROY from scope exit should not die +pass("double DESTROY did not die"); + +# --- guard unmocks even if test dies (eval) --- +eval { + my $guard = mock_file_check_guard( '-e' => sub { CHECK_IS_TRUE } ); + ok( -e $fake, "mocked -e inside eval" ); + die "simulated test failure"; +}; +ok( !-e $fake, "-e unmocked after die inside eval" ); + +# --- guard works with dash-less check names --- +{ + my $guard = mock_file_check_guard( 'e' => sub { CHECK_IS_TRUE } ); + ok( -e $fake, "mocked with dash-less 'e'" ); +} +ok( !-e $fake, "unmocked after dash-less guard" ); + +# --- guard with FALLBACK_TO_REAL_OP --- +{ + my $guard = mock_file_check_guard( '-e' => sub { FALLBACK_TO_REAL_OP } ); + ok( !-e $fake, "FALLBACK_TO_REAL_OP falls through to real check" ); +} + +done_testing;