Skip to content

Commit

Permalink
Fix infinite loop when deleting multiple packed refs.
Browse files Browse the repository at this point in the history
It was stupid to link the same element twice to lock_file_list
and end up in a loop, so we certainly need a fix.

But it is not like we are taking a lock on multiple files in
this case.  It is just that we leave the linked element on the
list even after commit_lock_file() successfully removes the
cruft.

We cannot remove the list element in commit_lock_file(); if we
are interrupted in the middle of list manipulation, the call to
remove_lock_file_on_signal() will happen with a broken list
structure pointed by lock_file_list, which would cause the cruft
to remain, so not removing the list element is the right thing
to do.  Instead we should be reusing the element already on the
list.

There is already a code for that in lock_file() function in
lockfile.c.  The code checks lk->next and the element is linked
only when it is not already on the list -- which is incorrect
for the last element on the list (which has NULL in its next
field), but if you read the check as "is this element already on
the list?" it actually makes sense.  We do not want to link it
on the list again, nor we would want to set up signal/atexit
over and over.

Signed-off-by: Junio C Hamano <junkio@cox.net>
  • Loading branch information
Junio C Hamano committed Jan 3, 2007
1 parent e6d40d6 commit 1084b84
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 2 deletions.
1 change: 1 addition & 0 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ extern int refresh_cache(unsigned int flags);

struct lock_file {
struct lock_file *next;
char on_list;
char filename[PATH_MAX];
};
extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
Expand Down
7 changes: 6 additions & 1 deletion lockfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,21 @@ static int lock_file(struct lock_file *lk, const char *path)
sprintf(lk->filename, "%s.lock", path);
fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 <= fd) {
if (!lk->next) {
if (!lk->on_list) {
lk->next = lock_file_list;
lock_file_list = lk;
lk->on_list = 1;
}
if (lock_file_list) {
signal(SIGINT, remove_lock_file_on_signal);
atexit(remove_lock_file);
}
if (adjust_shared_perm(lk->filename))
return error("cannot fix permission bits on %s",
lk->filename);
}
else
lk->filename[0] = 0;
return fd;
}

Expand Down
1 change: 0 additions & 1 deletion refs.c
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,6 @@ static int repack_without_ref(const char *refname)
}
if (!found)
return 0;
memset(&packlock, 0, sizeof(packlock));
fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
if (fd < 0)
return error("cannot delete '%s' from packed refs", refname);
Expand Down

0 comments on commit 1084b84

Please sign in to comment.