Skip Menu |
 

This queue is for tickets about the File-Path CPAN distribution.

Report information
The Basics
Id: 99230
Status: resolved
Priority: 0/
Queue: File-Path

People
Owner: RICHE [...] cpan.org
Requestors: jim.avera [...] gmail.com
Cc: jkeenan [...] cpan.org
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



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>
Download (untitled) / with headers
text/plain 967b
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".
Download (untitled) / with headers
text/plain 1.1k
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.
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>
Download (untitled) / with headers
text/plain 643b
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++; }
Download (untitled) / with headers
text/plain 566b
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
Download (untitled) / with headers
text/plain 156b
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.
Download (untitled) / with headers
text/plain 489b
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.
Download (untitled) / with headers
text/plain 217b
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.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.