repository corruption when pushing commits to a repository running 'git gc --prune='

Hi

I've recently expired my reflog to prune loose objects. On a live, bare, repository I ran 'git gc --prune=now'

All clients ended up having problems, they would report:
  error: refs/heads/master does not point to a valid object!
Running 'git log' on the bare repo gave : fatal: bad object HEAD

So I did a couple of tries reproducing the issue and with this little test I got a couple of interesting different/similar outcomes

A: mkdir -p /tmp/test; cd /tmp/test ; git init --bare bare ;
B: cd /tmp/test ; git clone bare 1 ; cd 1;  touch test; git add test; 
while true; do date >> test ; git commit -m "$(date)" test ; git push || 
break; done
A: cd bare; git gc --prune=now  (or run it in a loop)

1)

fatal: bad object 22f0351258fa0bb4cd28984b6473510957fbce69
fatal: bad object 22f0351258fa0bb4cd28984b6473510957fbce69
To /tmp/test/bare
  ! [remote rejected] master -> master (missing necessary objects)

2)

remote: error: unable to write sha1 filename 
objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed: No such file or directory
remote: fatal: failed to write object
error: unpack failed: unpack-objects abnormal exit
To /tmp/test/bare
  ! [remote rejected] master -> master (unpacker error)

3)

error: Ref refs/heads/master is at 
e992810e70949e797d33041bf6bc961c9fa4f3e5 but expected 
0000000000000000000000000000000000000000
remote: error: Cannot lock ref 'refs/heads/master':
To /tmp/test/bare
  ! [remote rejected] master -> master (failed to update ref)

4)

remote: error: cannot lock ref 'refs/heads/master': Unable to create 
'/tmp/test/bare/refs/heads/master.lock': File exists.
remote:
remote: If no other git process is currently running, this probably means a
remote: git process crashed in this repository earlier. Make sure no 
other git
remote: process is running and remove the file manually to continue.
To /tmp/test/bare
  ! [remote rejected] master -> master (failed to update ref)
error: failed to push some refs to '/tmp/test/bare'

And eventually running 'git gc --prune=now/all' on the bare repository could end with this message:

$ git gc --prune=all error: refs/heads/master does not point to a valid object! fatal: bad object refs/heads/master error: failed to run repack $ git log -1 fatal: bad object HEAD

This behaviour has been observed with version 2.4.4, 2.5.[12] and 2.6.1

Thank you

- Jan

Jan Smets <jan.smets@alcatel-lucent.com> writes:

[...]

I think this is the expected behavior. push will create new objects that are not referenced until the ref is updated (at the very end). prune can run concurrently since creating and deleting objects is done in a lockless way (only the ref update needs a lock).

Still, this is not the *documented* behavior, and an easy way to corrupt a repo should be very explicitly documented as very dangerous, and the precautions to take when using it should be explained clearly.

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Yeah, I think this paragraph in the user-manual is the only thing that mentions it:

    Anyway, once you are sure that you're not interested in any dangling
    state, you can just prune all unreachable objects:

    ------------------------------------------------
    $ git prune
    ------------------------------------------------

    and they'll be gone. (You should only run `git prune` on a quiescent
    repository--it's kind of like doing a filesystem fsck recovery: you
    don't want to do that while the filesystem is mounted.

Something along this line, perhaps?

 Documentation/git-gc.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Junio C Hamano <gitster@pobox.com> writes:

Looks good to me. I think the same should be added to git-prune.txt under --expire.

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

I thought about it, but decided against it, as the command is not even recommended to end users.

Junio C Hamano <gitster@pobox.com> writes:

Even non-"end users" can shoot themselves in the foot. Someone writting a script using "git prune" may actually shoot in other people's foot ...

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Sure, patches welcome.

To be honest, I felt that

    --expire <time>::
            Only expire loose objects older than <time>.

is sufficiently clear for these people.

Jan Smets <jan.smets@alcatel-lucent.com> writes:

If I read correctly, this happens when we move the temp file to its permanent position.

This looks like a race between deleting objects/05/ and creating objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed.

I don't understand how this is possible, since the temporary file and the final one are in the same directory, so deleting the directory should fail.

What am I missing?

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

 1. tmp_obj_* gets created in objects/05/ by "git unpack-objects";

 2. prune_cruft() comes, notices tmp_obj_* that is sufficiently old,
    decides to remove in "git prune".

 3. prune_subdir() comes, notices objects/05/ is now empty, does
    rmdir().

 4. "git unpack-objects" closes the file whose inode was originally
    associated with the path objects/05/tmp_obj_*;

 5. "git unpack-objects" calls finalize_object_file() and tries to
    link(2) or rename(2) it to its final name.  It does not find the
    source of the link or rename and says "oops, no such file".

Junio C Hamano <gitster@pobox.com> writes:

Ah, OK, the real problem is the temporary file removal. Thanks.