* bug#24714: delete-directory race condition
@ 2016-10-17 2:20 Glenn Morris
2016-10-17 6:19 ` Eli Zaretskii
2016-10-18 16:44 ` Paul Eggert
0 siblings, 2 replies; 16+ messages in thread
From: Glenn Morris @ 2016-10-17 2:20 UTC (permalink / raw)
To: 24714
Package: emacs
Version: 25.1
On current Debian testing, many tests in package-test.el fail for me with:
Test package-test-update-listing condition:
(file-error "Removing old name" "No such file or directory"
"/tmp/pkg-test-user-dir-27293kBj/gnupg/S.gpg-agent.rstrd")
I believe this is due to a race condition in delete-directory.
Emacs seems to be lacking an equivalent of "rm -rf".
(delete-directory "/tmp/foo")
will fail with "No such file or directory" if a file in /tmp/foo
happen to be deleted by some other process in between the time that
delete-directory calls directory-files and the time it calls delete-file.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-17 2:20 bug#24714: delete-directory race condition Glenn Morris
@ 2016-10-17 6:19 ` Eli Zaretskii
2016-10-17 15:52 ` Glenn Morris
2016-10-18 16:44 ` Paul Eggert
1 sibling, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2016-10-17 6:19 UTC (permalink / raw)
To: Glenn Morris; +Cc: 24714
> From: Glenn Morris <rgm@gnu.org>
> Date: Sun, 16 Oct 2016 22:20:45 -0400
>
> I believe this is due to a race condition in delete-directory.
> Emacs seems to be lacking an equivalent of "rm -rf".
>
> (delete-directory "/tmp/foo")
>
> will fail with "No such file or directory" if a file in /tmp/foo
> happen to be deleted by some other process in between the time that
> delete-directory calls directory-files and the time it calls delete-file.
IMO, delete-directory should simply catch ENOENT errors and ignore
them when it deletes files and subdirectories under the "recursive"
option. Other errors should signal an error as they do now.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-17 6:19 ` Eli Zaretskii
@ 2016-10-17 15:52 ` Glenn Morris
2016-10-17 16:11 ` Eli Zaretskii
2016-10-17 17:00 ` Andreas Schwab
0 siblings, 2 replies; 16+ messages in thread
From: Glenn Morris @ 2016-10-17 15:52 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 24714
Eli Zaretskii wrote:
>> will fail with "No such file or directory" if a file in /tmp/foo
>> happen to be deleted by some other process in between the time that
>> delete-directory calls directory-files and the time it calls delete-file.
>
> IMO, delete-directory should simply catch ENOENT errors and ignore
> them when it deletes files and subdirectories under the "recursive"
> option.
I don't think that's enough, since a file could equally well be
_created_ by some other process after delete-directory calls directory-files.
Frankly I don't see how Emacs's delete-directory can work reliably as
currently implemented.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-17 15:52 ` Glenn Morris
@ 2016-10-17 16:11 ` Eli Zaretskii
2016-10-17 17:00 ` Andreas Schwab
1 sibling, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2016-10-17 16:11 UTC (permalink / raw)
To: Glenn Morris; +Cc: 24714
> From: Glenn Morris <rgm@gnu.org>
> Cc: 24714@debbugs.gnu.org
> Date: Mon, 17 Oct 2016 11:52:31 -0400
>
> Eli Zaretskii wrote:
>
> >> will fail with "No such file or directory" if a file in /tmp/foo
> >> happen to be deleted by some other process in between the time that
> >> delete-directory calls directory-files and the time it calls delete-file.
> >
> > IMO, delete-directory should simply catch ENOENT errors and ignore
> > them when it deletes files and subdirectories under the "recursive"
> > option.
>
> I don't think that's enough, since a file could equally well be
> _created_ by some other process after delete-directory calls directory-files.
It will solve your use case, with files under /tmp, won't it? A
partial solution is better than no solution at all, IMO.
> Frankly I don't see how Emacs's delete-directory can work reliably as
> currently implemented.
Patches to reimplement it are also welcome.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-17 15:52 ` Glenn Morris
2016-10-17 16:11 ` Eli Zaretskii
@ 2016-10-17 17:00 ` Andreas Schwab
2016-10-18 16:50 ` Glenn Morris
1 sibling, 1 reply; 16+ messages in thread
From: Andreas Schwab @ 2016-10-17 17:00 UTC (permalink / raw)
To: Glenn Morris; +Cc: 24714
On Okt 17 2016, Glenn Morris <rgm@gnu.org> wrote:
> I don't think that's enough, since a file could equally well be
> _created_ by some other process after delete-directory calls directory-files.
That is ok. The operation cannot successfully be completed in this case.
> Frankly I don't see how Emacs's delete-directory can work reliably as
> currently implemented.
A property it shares with any other operation recursing on directories,
in Emacs or elsewhere.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-17 2:20 bug#24714: delete-directory race condition Glenn Morris
2016-10-17 6:19 ` Eli Zaretskii
@ 2016-10-18 16:44 ` Paul Eggert
2016-10-18 16:55 ` Glenn Morris
1 sibling, 1 reply; 16+ messages in thread
From: Paul Eggert @ 2016-10-18 16:44 UTC (permalink / raw)
To: Glenn Morris; +Cc: 24714
[-- Attachment #1: Type: text/plain, Size: 193 bytes --]
Although I didn't reproduce the bug, I changed delete-directory
according to Eli's suggestion by installing the attached patch into
master. Glenn, can you please try it in your environment?
[-- Attachment #2: 0001-delete-directory-no-longer-errors-when-racing.patch --]
[-- Type: application/x-patch, Size: 4740 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-17 17:00 ` Andreas Schwab
@ 2016-10-18 16:50 ` Glenn Morris
0 siblings, 0 replies; 16+ messages in thread
From: Glenn Morris @ 2016-10-18 16:50 UTC (permalink / raw)
To: Andreas Schwab; +Cc: 24714
Andreas Schwab wrote:
> That is ok. The operation cannot successfully be completed in this case.
>
>> Frankly I don't see how Emacs's delete-directory can work reliably as
>> currently implemented.
>
> A property it shares with any other operation recursing on directories,
> in Emacs or elsewhere.
That's interesting. I'd assumed there was a "Right Way" to do it, and
that it would be whatever coreutil's rm did. If not then we are back to:
> delete-directory should simply catch ENOENT errors and ignore them
> when it deletes files and subdirectories under the "recursive" option.
> Other errors should signal an error as they do now.
This doesn't seem possible without changes at the C level.
Should there be a standard Lisp error for ENOENT?
Currently report_file_errno only does EEXIST -> file_already_exists.
Or should delete-file get a FORCE argument akin to "rm -f"?
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-18 16:44 ` Paul Eggert
@ 2016-10-18 16:55 ` Glenn Morris
2016-10-18 16:59 ` Glenn Morris
2016-10-21 20:07 ` Paul Eggert
0 siblings, 2 replies; 16+ messages in thread
From: Glenn Morris @ 2016-10-18 16:55 UTC (permalink / raw)
To: Paul Eggert; +Cc: 24714
I haven't tested it, but the files--force implementation looks a little
unaesthetic to me. Did you consider adding a standard condition error in
report_file_errno for ENOENT? There would then be symmetry with the
file-already-exists error for EEXIST.
Also, IMO this is purely a bug fix, and should not be mentioned in NEWS,
the lispref, or the doc.
But thanks for the fix!
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-18 16:55 ` Glenn Morris
@ 2016-10-18 16:59 ` Glenn Morris
2016-10-19 7:23 ` Eli Zaretskii
2016-10-21 20:07 ` Paul Eggert
1 sibling, 1 reply; 16+ messages in thread
From: Glenn Morris @ 2016-10-18 16:59 UTC (permalink / raw)
To: Paul Eggert; +Cc: 24714
Glenn Morris wrote:
> I haven't tested it, but the files--force implementation looks a little
> unaesthetic to me.
PS will it work in non-English locales?
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-18 16:59 ` Glenn Morris
@ 2016-10-19 7:23 ` Eli Zaretskii
2016-10-20 6:50 ` Paul Eggert
2016-10-21 11:53 ` Daniel Colascione
0 siblings, 2 replies; 16+ messages in thread
From: Eli Zaretskii @ 2016-10-19 7:23 UTC (permalink / raw)
To: Glenn Morris; +Cc: eggert, 24714
> From: Glenn Morris <rgm@gnu.org>
> Date: Tue, 18 Oct 2016 12:59:26 -0400
> Cc: 24714@debbugs.gnu.org
>
> Glenn Morris wrote:
>
> > I haven't tested it, but the files--force implementation looks a little
> > unaesthetic to me.
>
> PS will it work in non-English locales?
Paul, what about this Glenn's question? The error strings are
localized, I think.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-19 7:23 ` Eli Zaretskii
@ 2016-10-20 6:50 ` Paul Eggert
2016-10-21 11:53 ` Daniel Colascione
1 sibling, 0 replies; 16+ messages in thread
From: Paul Eggert @ 2016-10-20 6:50 UTC (permalink / raw)
To: Eli Zaretskii, Glenn Morris; +Cc: 24714
Eli Zaretskii wrote:
> Paul, what about this Glenn's question? The error strings are
> localized, I think.
Yes, his comments are spot on. I have written a fix but want to test it more
before installing.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-19 7:23 ` Eli Zaretskii
2016-10-20 6:50 ` Paul Eggert
@ 2016-10-21 11:53 ` Daniel Colascione
2016-10-21 12:09 ` Andreas Schwab
2016-10-21 12:55 ` Eli Zaretskii
1 sibling, 2 replies; 16+ messages in thread
From: Daniel Colascione @ 2016-10-21 11:53 UTC (permalink / raw)
To: Eli Zaretskii, Glenn Morris; +Cc: eggert, 24714
On 10/19/2016 12:23 AM, Eli Zaretskii wrote:
>> From: Glenn Morris <rgm@gnu.org>
>> Date: Tue, 18 Oct 2016 12:59:26 -0400
>> Cc: 24714@debbugs.gnu.org
>>
>> Glenn Morris wrote:
>>
>>> I haven't tested it, but the files--force implementation looks a little
>>> unaesthetic to me.
>>
>> PS will it work in non-English locales?
>
> Paul, what about this Glenn's question? The error strings are
> localized, I think.
What about attaching the numeric errno value to the error message as a
text property in report_file_errno?
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-21 11:53 ` Daniel Colascione
@ 2016-10-21 12:09 ` Andreas Schwab
2016-10-21 12:10 ` Daniel Colascione
2016-10-21 12:55 ` Eli Zaretskii
1 sibling, 1 reply; 16+ messages in thread
From: Andreas Schwab @ 2016-10-21 12:09 UTC (permalink / raw)
To: Daniel Colascione; +Cc: eggert, 24714
On Okt 21 2016, Daniel Colascione <dancol@dancol.org> wrote:
> What about attaching the numeric errno value to the error message as a
> text property in report_file_errno?
errno values aren't portable either.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-21 12:09 ` Andreas Schwab
@ 2016-10-21 12:10 ` Daniel Colascione
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Colascione @ 2016-10-21 12:10 UTC (permalink / raw)
To: Andreas Schwab; +Cc: eggert, 24714
On 10/21/2016 05:09 AM, Andreas Schwab wrote:
> On Okt 21 2016, Daniel Colascione <dancol@dancol.org> wrote:
>
>> What about attaching the numeric errno value to the error message as a
>> text property in report_file_errno?
>
> errno values aren't portable either.
True, but you can provide the value of EEXIST and others as constants
accessible to lisp.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-21 11:53 ` Daniel Colascione
2016-10-21 12:09 ` Andreas Schwab
@ 2016-10-21 12:55 ` Eli Zaretskii
1 sibling, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2016-10-21 12:55 UTC (permalink / raw)
To: Daniel Colascione; +Cc: eggert, 24714
> Cc: eggert@cs.ucla.edu, 24714@debbugs.gnu.org
> From: Daniel Colascione <dancol@dancol.org>
> Date: Fri, 21 Oct 2016 04:53:40 -0700
>
> > Paul, what about this Glenn's question? The error strings are
> > localized, I think.
>
> What about attaching the numeric errno value to the error message as a
> text property in report_file_errno?
IMO, Glenn suggested a cleaner solution: to have a symbol for ENOENT.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#24714: delete-directory race condition
2016-10-18 16:55 ` Glenn Morris
2016-10-18 16:59 ` Glenn Morris
@ 2016-10-21 20:07 ` Paul Eggert
1 sibling, 0 replies; 16+ messages in thread
From: Paul Eggert @ 2016-10-21 20:07 UTC (permalink / raw)
To: Glenn Morris; +Cc: 24714
[-- Attachment #1: Type: text/plain, Size: 361 bytes --]
On 10/18/2016 09:55 AM, Glenn Morris wrote:
> Did you consider adding a standard condition error in
> report_file_errno for ENOENT? There would then be symmetry with the
> file-already-exists error for EEXIST.
Again, thanks for pointing that out. I installed the attached patch,
which does that. This also should fix the locale problem you mentioned
later.
[-- Attachment #2: 0001-New-error-file-missing.patch --]
[-- Type: application/x-patch, Size: 16007 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-10-21 20:07 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-17 2:20 bug#24714: delete-directory race condition Glenn Morris
2016-10-17 6:19 ` Eli Zaretskii
2016-10-17 15:52 ` Glenn Morris
2016-10-17 16:11 ` Eli Zaretskii
2016-10-17 17:00 ` Andreas Schwab
2016-10-18 16:50 ` Glenn Morris
2016-10-18 16:44 ` Paul Eggert
2016-10-18 16:55 ` Glenn Morris
2016-10-18 16:59 ` Glenn Morris
2016-10-19 7:23 ` Eli Zaretskii
2016-10-20 6:50 ` Paul Eggert
2016-10-21 11:53 ` Daniel Colascione
2016-10-21 12:09 ` Andreas Schwab
2016-10-21 12:10 ` Daniel Colascione
2016-10-21 12:55 ` Eli Zaretskii
2016-10-21 20:07 ` Paul Eggert
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.