Skip to content

Commit 39cd1e8

Browse files
committed
Clone dirhandles without fchdir
This uses fdopendir and dup to dirhandles. This means it won't change working directory during thread cloning, which prevents race conditions that can happen if a third thread is active at the same time.
1 parent ab79bb2 commit 39cd1e8

File tree

12 files changed

+28
-191
lines changed

12 files changed

+28
-191
lines changed

Configure

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,7 @@ d_fd_set=''
478478
d_fds_bits=''
479479
d_fdclose=''
480480
d_fdim=''
481+
d_fdopendir=''
481482
d_fegetround=''
482483
d_ffs=''
483484
d_ffsl=''
@@ -13344,6 +13345,10 @@ esac
1334413345
set i_fcntl
1334513346
eval $setvar
1334613347

13348+
: see if fdopendir exists
13349+
set fdopendir d_fdopendir
13350+
eval $inlibc
13351+
1334713352
: see if fork exists
1334813353
set fork d_fork
1334913354
eval $inlibc
@@ -25052,6 +25057,7 @@ d_flockproto='$d_flockproto'
2505225057
d_fma='$d_fma'
2505325058
d_fmax='$d_fmax'
2505425059
d_fmin='$d_fmin'
25060+
d_fdopendir='$d_fdopendir'
2505525061
d_fork='$d_fork'
2505625062
d_fp_class='$d_fp_class'
2505725063
d_fp_classify='$d_fp_classify'

Cross/config.sh-arm-linux

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ d_fd_macros='define'
212212
d_fd_set='define'
213213
d_fdclose='undef'
214214
d_fdim='undef'
215+
d_fdopendir=undef
215216
d_fds_bits='undef'
216217
d_fegetround='define'
217218
d_ffs='undef'

Cross/config.sh-arm-linux-n770

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ d_fd_macros='define'
211211
d_fd_set='define'
212212
d_fdclose='undef'
213213
d_fdim='undef'
214+
d_fdopendir=undef
214215
d_fds_bits='undef'
215216
d_fegetround='define'
216217
d_ffs='undef'

Porting/Glossary

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,11 @@ d_fmin (d_fmin.U):
947947
This variable conditionally defines the HAS_FMIN symbol, which
948948
indicates to the C program that the fmin() routine is available.
949949

950+
d_fdopendir (d_fdopendir.U):
951+
This variable conditionally defines the HAS_FORK symbol, which
952+
indicates that the fdopen routine is available to open a
953+
directory descriptor.
954+
950955
d_fork (d_fork.U):
951956
This variable conditionally defines the HAS_FORK symbol, which
952957
indicates to the C program that the fork() routine is available.

Porting/config.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ d_fd_macros='define'
223223
d_fd_set='define'
224224
d_fdclose='undef'
225225
d_fdim='define'
226+
d_fdopendir='define'
226227
d_fds_bits='define'
227228
d_fegetround='define'
228229
d_ffs='define'

config_h.SH

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,12 @@ sed <<!GROK!THIS! >$CONFIG_H -e 's!^#undef\(.*/\)\*!/\*#define\1 \*!' -e 's!^#un
142142
*/
143143
#$d_fcntl HAS_FCNTL /**/
144144
145+
/* HAS_FDOPENDIR:
146+
* This symbol, if defined, indicates that the fdopen routine is
147+
* available to open a directory descriptor.
148+
*/
149+
#$d_fdopendir HAS_FDOPENDIR /**/
150+
145151
/* HAS_FGETPOS:
146152
* This symbol, if defined, indicates that the fgetpos routine is
147153
* available to get the file position indicator, similar to ftell().

configure.com

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6010,6 +6010,7 @@ $ WC "d_fd_set='" + d_fd_set + "'"
60106010
$ WC "d_fd_macros='define'"
60116011
$ WC "d_fdclose='undef'"
60126012
$ WC "d_fdim='" + d_fdim + "'"
6013+
$ WC "d_fdopendir='undef'"
60136014
$ WC "d_fds_bits='define'"
60146015
$ WC "d_fegetround='undef'"
60156016
$ WC "d_ffs='undef'"

plan9/config_sh.sample

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ d_fd_macros='undef'
212212
d_fd_set='undef'
213213
d_fdclose='undef'
214214
d_fdim='undef'
215+
d_fdopendir=undef
215216
d_fds_bits='undef'
216217
d_fegetround='undef'
217218
d_ffs='undef'

sv.c

Lines changed: 3 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -14096,15 +14096,6 @@ Perl_dirp_dup(pTHX_ DIR *const dp, CLONE_PARAMS *const param)
1409614096
{
1409714097
DIR *ret;
1409814098

14099-
#if defined(HAS_FCHDIR) && defined(HAS_TELLDIR) && defined(HAS_SEEKDIR)
14100-
DIR *pwd;
14101-
const Direntry_t *dirent;
14102-
char smallbuf[256]; /* XXX MAXPATHLEN, surely? */
14103-
char *name = NULL;
14104-
STRLEN len = 0;
14105-
long pos;
14106-
#endif
14107-
1410814099
PERL_UNUSED_CONTEXT;
1410914100
PERL_ARGS_ASSERT_DIRP_DUP;
1411014101

@@ -14116,89 +14107,13 @@ Perl_dirp_dup(pTHX_ DIR *const dp, CLONE_PARAMS *const param)
1411614107
if (ret)
1411714108
return ret;
1411814109

14119-
#if defined(HAS_FCHDIR) && defined(HAS_TELLDIR) && defined(HAS_SEEKDIR)
14110+
#ifdef HAS_FDOPENDIR
1412014111

1412114112
PERL_UNUSED_ARG(param);
1412214113

14123-
/* create anew */
14124-
14125-
/* open the current directory (so we can switch back) */
14126-
if (!(pwd = PerlDir_open("."))) return (DIR *)NULL;
14127-
14128-
/* chdir to our dir handle and open the present working directory */
14129-
if (fchdir(my_dirfd(dp)) < 0 || !(ret = PerlDir_open("."))) {
14130-
PerlDir_close(pwd);
14131-
return (DIR *)NULL;
14132-
}
14133-
/* Now we should have two dir handles pointing to the same dir. */
14134-
14135-
/* Be nice to the calling code and chdir back to where we were. */
14136-
/* XXX If this fails, then what? */
14137-
PERL_UNUSED_RESULT(fchdir(my_dirfd(pwd)));
14114+
ret = fdopendir(dup(my_dirfd(dp)));
1413814115

14139-
/* We have no need of the pwd handle any more. */
14140-
PerlDir_close(pwd);
14141-
14142-
#ifdef DIRNAMLEN
14143-
# define d_namlen(d) (d)->d_namlen
14144-
#else
14145-
# define d_namlen(d) strlen((d)->d_name)
14146-
#endif
14147-
/* Iterate once through dp, to get the file name at the current posi-
14148-
tion. Then step back. */
14149-
pos = PerlDir_tell(dp);
14150-
if ((dirent = PerlDir_read(dp))) {
14151-
len = d_namlen(dirent);
14152-
if (len > sizeof(dirent->d_name) && sizeof(dirent->d_name) > PTRSIZE) {
14153-
/* If the len is somehow magically longer than the
14154-
* maximum length of the directory entry, even though
14155-
* we could fit it in a buffer, we could not copy it
14156-
* from the dirent. Bail out. */
14157-
PerlDir_close(ret);
14158-
return (DIR*)NULL;
14159-
}
14160-
if (len <= sizeof smallbuf) name = smallbuf;
14161-
else Newx(name, len, char);
14162-
Move(dirent->d_name, name, len, char);
14163-
}
14164-
PerlDir_seek(dp, pos);
14165-
14166-
/* Iterate through the new dir handle, till we find a file with the
14167-
right name. */
14168-
if (!dirent) /* just before the end */
14169-
for(;;) {
14170-
pos = PerlDir_tell(ret);
14171-
if (PerlDir_read(ret)) continue; /* not there yet */
14172-
PerlDir_seek(ret, pos); /* step back */
14173-
break;
14174-
}
14175-
else {
14176-
const long pos0 = PerlDir_tell(ret);
14177-
for(;;) {
14178-
pos = PerlDir_tell(ret);
14179-
if ((dirent = PerlDir_read(ret))) {
14180-
if (len == (STRLEN)d_namlen(dirent)
14181-
&& memEQ(name, dirent->d_name, len)) {
14182-
/* found it */
14183-
PerlDir_seek(ret, pos); /* step back */
14184-
break;
14185-
}
14186-
/* else we are not there yet; keep iterating */
14187-
}
14188-
else { /* This is not meant to happen. The best we can do is
14189-
reset the iterator to the beginning. */
14190-
PerlDir_seek(ret, pos0);
14191-
break;
14192-
}
14193-
}
14194-
}
14195-
#undef d_namlen
14196-
14197-
if (name && name != smallbuf)
14198-
Safefree(name);
14199-
#endif
14200-
14201-
#ifdef WIN32
14116+
#elif defined(WIN32)
1420214117
ret = win32_dirp_dup(dp, param);
1420314118
#endif
1420414119

t/op/threads-dirh.t

Lines changed: 1 addition & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,12 @@ BEGIN {
1313
skip_all_if_miniperl("no dynamic loading on miniperl, no threads");
1414
skip_all("runs out of memory on some EBCDIC") if $ENV{PERL_SKIP_BIG_MEM_TESTS};
1515

16-
plan(6);
16+
plan(1);
1717
}
1818

1919
use strict;
2020
use warnings;
2121
use threads;
22-
use threads::shared;
23-
use File::Path;
24-
use File::Spec::Functions qw 'updir catdir';
25-
use Cwd 'getcwd';
2622

2723
# Basic sanity check: make sure this does not crash
2824
fresh_perl_is <<'# this is no comment', 'ok', {}, 'crash when duping dirh';
@@ -31,101 +27,3 @@ fresh_perl_is <<'# this is no comment', 'ok', {}, 'crash when duping dirh';
3127
async{}->join for 1..2;
3228
print "ok";
3329
# this is no comment
34-
35-
my $dir;
36-
SKIP: {
37-
skip "telldir or seekdir not defined on this platform", 5
38-
if !$Config::Config{d_telldir} || !$Config::Config{d_seekdir};
39-
my $skip = sub {
40-
chdir($dir);
41-
chdir updir;
42-
skip $_[0], 5
43-
};
44-
45-
if(!$Config::Config{d_fchdir} && $^O ne "MSWin32") {
46-
$::TODO = 'dir handle cloning currently requires fchdir on non-Windows platforms';
47-
}
48-
49-
my @w :shared; # warnings accumulator
50-
local $SIG{__WARN__} = sub { push @w, $_[0] };
51-
52-
$dir = catdir getcwd(), "thrext$$" . int rand() * 100000;
53-
54-
rmtree($dir) if -d $dir;
55-
mkdir($dir);
56-
57-
# Create a dir structure like this:
58-
# $dir
59-
# |
60-
# `- toberead
61-
# |
62-
# +---- thrit
63-
# |
64-
# +---- rile
65-
# |
66-
# `---- zor
67-
68-
chdir($dir);
69-
mkdir 'toberead';
70-
chdir 'toberead';
71-
{open my $fh, ">thrit" or &$skip("Cannot create file thrit")}
72-
{open my $fh, ">rile" or &$skip("Cannot create file rile")}
73-
{open my $fh, ">zor" or &$skip("Cannot create file zor")}
74-
chdir updir;
75-
76-
# Then test that dir iterators are cloned correctly.
77-
78-
opendir my $toberead, 'toberead';
79-
my $start_pos = telldir $toberead;
80-
my @first_2 = (scalar readdir $toberead, scalar readdir $toberead);
81-
my @from_thread = @{; async { [readdir $toberead ] } ->join };
82-
my @from_main = readdir $toberead;
83-
is join('-', sort @from_thread), join('-', sort @from_main),
84-
'dir iterator is copied from one thread to another';
85-
like
86-
join('-', "", sort(@first_2, @from_thread), ""),
87-
qr/(?<!-rile)-rile-thrit-zor-(?!zor-)/i,
88-
'cloned iterator iterates exactly once over everything not already seen';
89-
90-
seekdir $toberead, $start_pos;
91-
readdir $toberead for 1 .. @first_2+@from_thread;
92-
{
93-
local $::TODO; # This always passes when dir handles are not cloned.
94-
is
95-
async { readdir $toberead // 'undef' } ->join, 'undef',
96-
'cloned dir iterator that points to the end of the directory'
97-
;
98-
}
99-
100-
# Make sure the cloning code can handle file names longer than 255 chars
101-
SKIP: {
102-
chdir 'toberead';
103-
open my $fh,
104-
">floccipaucinihilopilification-"
105-
. "pneumonoultramicroscopicsilicovolcanoconiosis-"
106-
. "lopadotemachoselachogaleokranioleipsanodrimypotrimmatosilphiokarabo"
107-
. "melitokatakechymenokichlepikossyphophattoperisteralektryonoptokephal"
108-
. "liokinklopeleiolagoiosiraiobaphetraganopterygon"
109-
or
110-
chdir updir,
111-
skip("OS does not support long file names (and I mean *long*)", 1);
112-
chdir updir;
113-
opendir my $dirh, "toberead";
114-
my $test_name
115-
= "dir iterators can be cloned when the next fn > 255 chars";
116-
while() {
117-
my $pos = telldir $dirh;
118-
my $fn = readdir($dirh);
119-
if(!defined $fn) { fail($test_name); last SKIP; }
120-
if($fn =~ 'lagoio') {
121-
seekdir $dirh, $pos;
122-
last;
123-
}
124-
}
125-
is length async { scalar readdir $dirh } ->join, 258, $test_name;
126-
}
127-
128-
is scalar @w, 0, 'no warnings during all that' or diag @w;
129-
chdir updir;
130-
}
131-
rmtree($dir);

0 commit comments

Comments
 (0)