From 4d516b3b1874df17f67bd33359fd3688304aa8d8 Mon Sep 17 00:00:00 2001 From: Toddr Bot Date: Thu, 21 May 2026 22:42:19 +0000 Subject: [PATCH] fix: support multiple concurrent opendir handles on same directory Opening the same mocked directory with multiple dirhandles caused the second opendir to overwrite the first handle's iteration state, making subsequent readdir/telldir/seekdir/rewinddir on the first handle fail. Replace the single-slot {'obj'}/{'fh'} storage with a per-handle {'dir_handles'} hash keyed by stringified dirhandle, giving each opendir call its own independent DirHandle with separate position. Co-Authored-By: Claude Opus 4.6 --- MANIFEST | 1 + lib/Test/MockFile.pm | 39 +++++++------- t/concurrent_opendir.t | 119 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 21 deletions(-) create mode 100644 t/concurrent_opendir.t diff --git a/MANIFEST b/MANIFEST index 0618c7e..46a18f1 100644 --- a/MANIFEST +++ b/MANIFEST @@ -24,6 +24,7 @@ t/chmod-filetemp.t t/chmod.t t/chown-chmod-nostrict.t t/chown.t +t/concurrent_opendir.t t/creation_timestamps.t t/cwd_abs_path.t t/detect-common-mistakes.t diff --git a/lib/Test/MockFile.pm b/lib/Test/MockFile.pm index 6c5d4cf..95364c7 100644 --- a/lib/Test/MockFile.pm +++ b/lib/Test/MockFile.pm @@ -1859,8 +1859,8 @@ sub _fh_to_file { } } - # Check dir handle (stored as stringified handle) - if ( $mock->{'fh'} && $mock->{'fh'} eq "$fh" ) { + # Check dir handles (multiple concurrent handles per directory) + if ( $mock->{'dir_handles'} && exists $mock->{'dir_handles'}{"$fh"} ) { return $path; } } @@ -3475,10 +3475,10 @@ sub __opendir (*$) { *{ $_[0] } = Symbol::geniosym; } - # This is how we tell if the file is open by something. - # $abs_path already holds the resolved path from _find_file_or_fh above. - $mock_dir->{'obj'} = Test::MockFile::DirHandle->new( $abs_path, $mock_dir->contents() ); - $mock_dir->{'fh'} = "$_[0]"; + # Track per-handle DirHandle objects so multiple concurrent opendir + # calls on the same directory each maintain independent iteration state. + $mock_dir->{'dir_handles'} //= {}; + $mock_dir->{'dir_handles'}{"$_[0]"} = Test::MockFile::DirHandle->new( $abs_path, $mock_dir->contents() ); return 1; @@ -3498,7 +3498,7 @@ sub __readdir (*) { return CORE::readdir( $_[0] ); } - my $obj = $mocked_dir->{'obj'}; + my $obj = $mocked_dir->{'dir_handles'} && $mocked_dir->{'dir_handles'}{"$_[0]"}; if ( !$obj ) { warnings::warnif( 'io', "readdir() attempted on invalid dirhandle $_[0]" ); return; @@ -3544,13 +3544,12 @@ sub __telldir (*) { return CORE::telldir($fh); } - if ( !$mocked_dir->{'obj'} ) { + my $obj = $mocked_dir->{'dir_handles'} && $mocked_dir->{'dir_handles'}{"$fh"}; + if ( !$obj ) { warnings::warnif( 'io', "telldir() attempted on invalid dirhandle $fh" ); return undef; } - my $obj = $mocked_dir->{'obj'}; - if ( !defined $obj->{'files_in_readdir'} ) { confess("Did a telldir on an empty dir. This shouldn't have been able to have been opened!"); } @@ -3577,13 +3576,12 @@ sub __rewinddir (*) { return CORE::rewinddir( $_[0] ); } - if ( !$mocked_dir->{'obj'} ) { + my $obj = $mocked_dir->{'dir_handles'} && $mocked_dir->{'dir_handles'}{"$fh"}; + if ( !$obj ) { warnings::warnif( 'io', "rewinddir() attempted on invalid dirhandle $fh" ); return; } - my $obj = $mocked_dir->{'obj'}; - if ( !defined $obj->{'files_in_readdir'} ) { confess("Did a rewinddir on an empty dir. This shouldn't have been able to have been opened!"); } @@ -3611,13 +3609,12 @@ sub __seekdir (*$) { return CORE::seekdir( $fh, $goto ); } - if ( !$mocked_dir->{'obj'} ) { + my $obj = $mocked_dir->{'dir_handles'} && $mocked_dir->{'dir_handles'}{"$fh"}; + if ( !$obj ) { warnings::warnif( 'io', "seekdir() attempted on invalid dirhandle $fh" ); return; } - my $obj = $mocked_dir->{'obj'}; - if ( !defined $obj->{'files_in_readdir'} ) { confess("Did a seekdir on an empty dir. This shouldn't have been able to have been opened!"); } @@ -3648,17 +3645,17 @@ sub __closedir (*) { return CORE::closedir($fh); } - # Already closed — warn and return EBADF like real closedir - if ( !$mocked_dir->{'obj'} ) { + # Already closed or never opened — warn and return EBADF like real closedir + my $dh_key = "$fh"; + if ( !$mocked_dir->{'dir_handles'} || !$mocked_dir->{'dir_handles'}{$dh_key} ) { warnings::warnif( 'io', "closedir() attempted on invalid dirhandle $fh" ); $! = EBADF; _maybe_throw_autodie( 'closedir', @_ ); return undef; } - delete $mocked_dir->{'obj'}; - - # Keep $mocked_dir->{'fh'} so double-close is detected as mock, not CORE + # Set to undef (keep key so double-close is detected as mock, not CORE) + $mocked_dir->{'dir_handles'}{$dh_key} = undef; return 1; } diff --git a/t/concurrent_opendir.t b/t/concurrent_opendir.t new file mode 100644 index 0000000..ba097fa --- /dev/null +++ b/t/concurrent_opendir.t @@ -0,0 +1,119 @@ +#!/usr/bin/perl -w + +use strict; +use warnings; + +use Test2::Bundle::Extended; +use Test2::Tools::Explain; + +use Test::MockFile qw< nostrict >; + +note "-------------- concurrent opendir: independent iteration state --------------"; +{ + my $dir = Test::MockFile->new_dir( '/cdir', { 'autovivify' => 1 } ); + + open my $fh, '>', '/cdir/aaa' or die; + close $fh; + open $fh, '>', '/cdir/bbb' or die; + close $fh; + + opendir my $dh1, '/cdir' or die "opendir1: $!"; + my $first1 = readdir($dh1); + is( $first1, '.', 'dh1 reads . first' ); + + opendir my $dh2, '/cdir' or die "opendir2: $!"; + my $first2 = readdir($dh2); + is( $first2, '.', 'dh2 reads . first' ); + + my $second1 = readdir($dh1); + is( $second1, '..', 'dh1 reads .. second (not clobbered by dh2)' ); + + my $second2 = readdir($dh2); + is( $second2, '..', 'dh2 reads .. second' ); + + my @rest1 = readdir($dh1); + my @rest2 = readdir($dh2); + is( \@rest1, [qw/aaa bbb/], 'dh1 reads remaining files' ); + is( \@rest2, [qw/aaa bbb/], 'dh2 reads remaining files independently' ); + + closedir $dh1; + closedir $dh2; +} + +note "-------------- concurrent opendir: close one, other continues --------------"; +{ + my $dir = Test::MockFile->new_dir('/cdir2'); + + opendir my $dh1, '/cdir2' or die; + opendir my $dh2, '/cdir2' or die; + + readdir($dh1); # . + closedir $dh1; + + my @entries = readdir($dh2); + is( \@entries, [ '.', '..' ], 'dh2 still has full iteration after dh1 is closed' ); + + closedir $dh2; +} + +note "-------------- concurrent opendir: telldir/seekdir per handle --------------"; +{ + my $dir = Test::MockFile->new_dir( '/cdir3', { 'autovivify' => 1 } ); + + open my $fh, '>', '/cdir3/x' or die; + close $fh; + + opendir my $dh1, '/cdir3' or die; + opendir my $dh2, '/cdir3' or die; + + readdir($dh1); # . + readdir($dh1); # .. + + is( telldir($dh1), 2, 'dh1 tell is 2 after reading two entries' ); + is( telldir($dh2), 0, 'dh2 tell is still 0' ); + + seekdir( $dh1, 0 ); + is( scalar readdir($dh1), '.', 'dh1 seeked back to start' ); + + is( scalar readdir($dh2), '.', 'dh2 unaffected by dh1 seek' ); + + closedir $dh1; + closedir $dh2; +} + +note "-------------- concurrent opendir: rewinddir per handle --------------"; +{ + my $dir = Test::MockFile->new_dir('/cdir4'); + + opendir my $dh1, '/cdir4' or die; + opendir my $dh2, '/cdir4' or die; + + readdir($dh1); # . + readdir($dh1); # .. + rewinddir($dh1); + + is( telldir($dh1), 0, 'dh1 rewound to 0' ); + is( telldir($dh2), 0, 'dh2 still at 0 (unaffected)' ); + is( scalar readdir($dh1), '.', 'dh1 reads . after rewind' ); + + closedir $dh1; + closedir $dh2; +} + +note "-------------- concurrent opendir: double close warns --------------"; +{ + my $dir = Test::MockFile->new_dir('/cdir5'); + + opendir my $dh, '/cdir5' or die; + closedir $dh; + + my @w; + { + local $SIG{__WARN__} = sub { push @w, $_[0] }; + closedir $dh; + } + + ok( @w == 1 && $w[0] =~ /closedir\(\) attempted on invalid dirhandle/, 'double close warns' ); +} + +done_testing;