libgit2/libgit2 — pull request #1866 by ethomson (2013-09-24 14:17:16+00:00)
Title: Avoid failing in fs_iterator race conditions

During filesystem iteration, a file may disappear between calls to `readdir` and subsequent calls to `lstat`, which would cause `lstat` to return of `GIT_ENOTFOUND`. Iterators and users overload `GIT_ENOTFOUND` to mean "empty directory". Unfortunately, confusing the aforementioned race condition with an "empty directory" is dangerous:

The error handling in `fs_iterator__expand_dir` will call `fs_iterator__advance_over` which memsets the `next` pointer to zero. Later the diff function (`diff.c`:932) will continue as if `GIT_ENOTFOUND` indicates empty directory and may dereference `next`.

This PR proposes that we indicate to callers that we cannot perform filesystem iteration because it has been changed. I propose returning `GIT_EAGAIN` to callers to indicate this race condition has occurred, appropriate here as `EAGAIN` is typically defined to mean "resource temporarily unavailable".

This, of course, means that callers will have to expect this (very non-fatal) error code and retry their operations which may be disappointing to the current users. It would be nice to have some of them weigh in on this as calling `git_status_xxx` functions or `git_diff_index_to_workdir` will now have to be protected and retried.

Another option is to simply remove entries that stopped existing from the path vector entirely. But I prefer this fail-fast mechanism to something that is likely to continue and provide inaccurate data.

(Note that there is no documentation about this new return code yet; I suspect this proposal will create some discussion, so I will document this if this is the direction we intend to take.)

/cc @arrbee
/cc @jamill