Bug #99230 for File-Path: Please document that File::Path::rmtree may not be used with threads

Tue Sep 30 14:24:36 2014 jim.avera [...] gmail.com - Ticket created

Subject:	Please document that File::Path::rmtree may not be used with threads
Date:	Tue, 30 Sep 2014 11:24:25 -0700
To:	bug-File-Path [...] rt.cpan.org
From:	Jim Avera <jim.avera [...] gmail.com>

File::Path::rmtree (v2.08) changes the working directory, which makes it incompatible with Perl threads (the cwd is process-global, shared by concurrent threads). PLEASE document this clearly in the pod to save people hours of unnecessary debugging. This was reported/requested last March in bug #94209. That bug received no comment from the maintainers. Presumably this means nobody has bandwidth to re-implement rmtree, which is perfectly understandable. But the request to at least document the limitation remains valid. I spent many long days debugging a multi-threaded app and would very much like to have been spared that useless exercise. Documenting the limitation would save many people lots of work. Thank you. P.S. bug#94209 has a comment "already been reported into perl RT without any reply" and referencing URL https://rt.perl.org/Public/Bug/Display.html?id=112008 but clicking that link gives error "Could not load ticket 112008".

Tue Sep 30 17:59:26 2014 ether [...] cpan.org - Correspondence added

On 2014-09-30 11:24:36, jim.avera@gmail.com wrote: Show quoted text

> File::Path::rmtree (v2.08) changes the working directory, which makes it > incompatible with Perl threads (the cwd is process-global, shared by > concurrent threads). > > PLEASE document this clearly in the pod to save people hours of > unnecessary debugging.

I'd prefer we die in this situation. No one reads documentation! (But the doc note should be there too!) Show quoted text

> This was reported/requested last March in bug #94209. That bug received > no comment from the maintainers. Presumably this means nobody has > bandwidth to re-implement rmtree, which is perfectly understandable. > But the request to at least document the limitation remains valid. I > spent many long days debugging a multi-threaded app and would very much > like to have been spared that useless exercise.

Fixing the implementation would be much preferable, of course, but in the meantime we can guard against accidents. Show quoted text

> P.S. bug#94209 has a comment "already been reported into perl RT without > any reply" and referencing URL > https://rt.perl.org/Public/Bug/Display.html?id=112008 but clicking that > link gives error "Could not load ticket 112008".

Works for me.

Tue Sep 30 17:59:26 2014 The RT System itself - Status changed from 'new' to 'open'

Tue Sep 30 18:38:26 2014 jim.avera [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #99230] Please document that File::Path::rmtree may not be used with threads
Date:	Tue, 30 Sep 2014 15:38:16 -0700
To:	bug-File-Path [...] rt.cpan.org
From:	Jim Avera <jim.avera [...] gmail.com>

On 09/30/2014 02:59 PM, Karen Etheridge via RT wrote: Show quoted text

> I'd prefer we die in this situation. No one reads documentation! (But > the doc note should be there too!)

Agree, dying when rmtree() is called with other threads would save a lot of pain. OTOH, an app _could_ arrange to block all but one thread while calling rmtree (arguably not a reasonable strategy, but possible). So maybe just warn: my $thread_warning_issued; # global And in rmtree() if (exists $INC{"threads.pm"}) { warn "WARNING: File::Path::rmtree is not thread safe (changes cwd)" if threads->list() > 0 && !$thread_warning_issued++; }

Thu Oct 02 20:52:25 2014 LEONT [...] cpan.org - Correspondence added

On Tue Sep 30 17:59:26 2014, ETHER wrote: Show quoted text

> I'd prefer we die in this situation. No one reads documentation! (But > the doc note should be there too!)

I don't think that is particularly useful behavior, specially because I don't see why this problem would be unfixable. Show quoted text

> Fixing the implementation would be much preferable, of course, but in > the meantime we can guard against accidents.

Given the backwards compatibility effect, I wouldn't call this a solution. It may have a race condition now, but that would not be an issue for all programs using this. Leon

Sat May 02 22:41:29 2015 BULKDD [...] cpan.org - Correspondence added

File::Path uses "." and File::Spec::Unix::curdir (aka ".") in many places. Maybe it can be rewritten to use only abs paths and then it never needs to chdir.

Fri Jun 26 12:13:10 2015 RICHE [...] cpan.org - Taken

Sat Jul 11 09:37:25 2015 jkeenan [...] cpan.org - Cc JKEENAN added

Sat Jul 11 09:39:00 2015 jkeenan [...] cpan.org - Reference to ticket #94209 added

Fri Jul 17 10:58:25 2015 RICHE [...] cpan.org - Correspondence added

I agree with both ether and bulk88 on the direction to resolve this issue. At this time we won't change any code relating to this issue and will document it and target a near term fix (dying) and long term fix (removing chdir) for the next release. Please see the following doc patch to mitigate the issue for the 2.10 release. https://github.com/rpcme/File-Path/commit/60c14331cfdf30980adbcd61ec2ac1fb1a65c4e5 As a side note, this issue won't be closed until the bug is fixed in code.

Sat Jul 18 00:06:13 2015 RICHE [...] cpan.org - Correspondence added

actually, we can resolve this as the 'documentation' problem as stated by OP, since 94209 is the ticket that really says it's not thread safe. This ticket will be referenced when we go to fix in code. Fixed in 2.11.

Sat Jul 18 00:06:15 2015 RICHE [...] cpan.org - Status changed from 'open' to 'resolved'