Junio C Hamano [off-list ref] writes:
I agree that this is a broken assumption on git's part. When we
have this loop:
while (ent = readdir()) {
... do something in that directory ...
}
we should expect readdir() may return new entries and cope with
it, as "If a file is removed from or added to the directory
after the most recent call to opendir() or rewinddir(), whether
a subsequent call to readdir() returns an entry for that file is
unspecified." [*1*]
Here is a result of a mini-audit.
* builtin-prune-packed.c::prune_dir() loops and unlinks (some
of) returned paths while in the loop. This should not
interfere with readdir(3). I am presuming that we can declare
readdir(3) implementation buggy if this happens:
* opendir();
* readdir() gives $P;
* unlink($P);
* readdir() later gives $P again.
Otherwise we need to lose check for return value from
unlink().
* builtin-prune.c::prune_dir() has a similar construct and the
same (non-)issue.
* dir.c::remove_dir_recursively() -- likewise.
* entry.c::remove_subtree() -- likewise. We might want to unify
this with the previous one.
A patch to "reflog-expire --all" will follow in a separate
message.
On Fri, 25 Jan 2008, Junio C Hamano wrote:
* builtin-prune-packed.c::prune_dir() loops and unlinks (some
of) returned paths while in the loop. This should not
interfere with readdir(3).
Correct. The "loop and unlink" behaviour is common and accepted practice,
and is, for example, what things like "rm -rf" tend to do.
It's only newly created files that may or may not show up in readdir()
after they've been created, regardless of whether they were created under
a name that was previously existed.
I am presuming that we can declare
readdir(3) implementation buggy if this happens:
* opendir();
* readdir() gives $P;
* unlink($P);
* readdir() later gives $P again.
Yes, but the potential problem is actually very different:
- directory contains 'a', 'b' and 'c'
* opendir()
* readdir() returns 'a'
* unlink('a');
* readdir() returns 'c', having skipped 'b'.
This is something that could in theory happen if a directory is indexed
using the *position* of a filename in a directory. 'a' was position 1, 'b'
was position 2, and 'c' was position 3. After the first readdir(), the
file position was 2 (pointing at 'b'), but when we removed 'a', the other
entries positions moved down, and now 'b' is at position 1, and 'c' is at
position '2'. When we call readdir() the next time, it skips 'b' (because
it already returned position 1!), and returns 'c'.
See? It's basically the issue of removing an entry in a linked list. And
yes, we've actually had bugs like that in our in-memory tmpfs. But they
were bugs, and they got fixed, and that's not how readdir() is supposed to
work, exactly because it makes doing an "rm -rf" very hard.
So readdir() should basically be safe wrt 'unlink()' happening while the
readdir() is running, and the only entry that can disappear is the one
that is unlinekd.
So don't worry about unlink's, it's only new files being created in the
same directory that can be problematic in that you don't know if
readdir() will return them or not..
Linus