On Tue, 2008-01-15 at 13:43 -0600, Brandon Casey wrote: > Linus Torvalds wrote: > > > > On Tue, 15 Jan 2008, Brandon Casey wrote: > >> Linus Torvalds wrote: > >>> It would obviously be interesting to see the base repository and the > >>> commit you are trying to do - is that possibly publicly available? > >> I wish it was. > > > > It's ok, I found the bug in your full strace. > > Good catch, but that wasn't it. Still getting the same error. > > > and now it's trying to close that fd *again* > > In that same vein, just above your changes in prepare_index() is: > > if (!pathspec || !*pathspec) { > fd = hold_locked_index(&index_lock, 1); > refresh_cache(REFRESH_QUIET); > if (write_cache(fd, active_cache, active_nr) || > close(fd) || commit_locked_index(&index_lock)) > die("unable to write new_index file"); > commit_style = COMMIT_AS_IS; > return get_index_file(); > } > > If I followed hold_locked_index() correctly, then fd and index_lock.fd > are equal, and commit_locked_index() does a close(lk->fd) making the > close(fd) above, redundant (or vice-versa). To my defense, the lockfile API is used a little inconsitently in git. Many places in git does a close(fd) and the call commit_locked_index(), which will close the fd again. Normally that will just cause an EBADFD which we ignore, but the problem here is that there's a longer time between close(fd) and the commit/rollback of the lock file. I guess the correct way to use the API is to never close the fd manually, but I copied and pasted the lockfile use in builtin-commit.c from somewhere else and along with it the double close. There's four close(fd) calls in prepare_index() and they're all incorrect. The open fd's are cleaned up in rollback_index_files() and shouldn't be closed manually. The patch below gets rid of the extra close() calls and should fix the problem. cheers, Kristian diff --git a/builtin-commit.c b/builtin-commit.c index 73f1e35..4494c9c 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -212,7 +212,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix) int fd = hold_locked_index(&index_lock, 1); add_files_to_cache(0, also ? prefix : NULL, pathspec); refresh_cache(REFRESH_QUIET); - if (write_cache(fd, active_cache, active_nr) || close(fd)) + if (write_cache(fd, active_cache, active_nr)) die("unable to write new_index file"); commit_style = COMMIT_NORMAL; return index_lock.filename; @@ -231,7 +231,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix) fd = hold_locked_index(&index_lock, 1); refresh_cache(REFRESH_QUIET); if (write_cache(fd, active_cache, active_nr) || - close(fd) || commit_locked_index(&index_lock)) + commit_locked_index(&index_lock)) die("unable to write new_index file"); commit_style = COMMIT_AS_IS; return get_index_file(); @@ -273,7 +273,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix) fd = hold_locked_index(&index_lock, 1); add_remove_files(&partial); refresh_cache(REFRESH_QUIET); - if (write_cache(fd, active_cache, active_nr) || close(fd)) + if (write_cache(fd, active_cache, active_nr)) die("unable to write new_index file"); fd = hold_lock_file_for_update(&false_lock, @@ -289,7 +289,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix) add_remove_files(&partial); refresh_cache(REFRESH_QUIET); - if (write_cache(fd, active_cache, active_nr) || close(fd)) + if (write_cache(fd, active_cache, active_nr)) die("unable to write temporary index file"); return false_lock.filename; } - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html