Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git-repack fails to delete an object directory it holds open #231

Closed
kostix opened this issue Jul 2, 2015 · 16 comments
Closed

git-repack fails to delete an object directory it holds open #231

kostix opened this issue Jul 2, 2015 · 16 comments

Comments

@kostix
Copy link

kostix commented Jul 2, 2015

If I do git gc --aggressive (with the recent RC4 on Windows XP SP3 32-bit), at a certain stage git repack tries to delete empty ".git/objects/NN" directories (after it supposedly moves the objects from them into a pack file), and fails.

The situation look slike this:

C:\devel\whatever>git gc --aggressive
Counting objects: 3163, done.
Compressing objects: 100% (3128/3128), done.
Writing objects: 100% (3163/3163), done.
Total 3163 (delta 2131), reused 1023 (delta 0)
Deletion of directory '.git/objects/01' failed. Should I try again? (y/n) y
Deletion of directory '.git/objects/01' failed. Should I try again? (y/n) y
Deletion of directory '.git/objects/01' failed. Should I try again? (y/n) y
Deletion of directory '.git/objects/01' failed. Should I try again? (y/n)

This happens for every other directory it tries to delete (if I answer "n" to the prompt).

The directories attempted to be deleted are truly empty -- I have verified that.

The details extracted using Process Explorer about the running git repack process are:

  • Executable's pathname: C:\Program Files\Git\mingw32\libexec\git-core\git.exe
  • Command-line: git repack -d -l -f --depth=250 --window=250 -A --unpack-unreachable=2.weeks.ago
  • Current directory: the repository's work tree.

If I use procexp to search through the handles currently open for items under ".git", procexp reveals that the handle to that repository is held by the same git repack process which is trying to delete it (it also has the second handle open -- to the packfile it's supposedly were placing the objects into).

@kostix
Copy link
Author

kostix commented Jul 2, 2015

Just in case, I have no antivirus software running on this box and have verified no other process has a handle open for those directories.

@kostix
Copy link
Author

kostix commented Jul 2, 2015

Here's the screenshot of procexp showing the handles it found for the filename pattern ".git":

git-repack-opened-handles

@dscho
Copy link
Member

dscho commented Jul 3, 2015

I just verified that there is no call to CreateFile() in compat/mingw.c that is not accompanied by a CloseHandle(). To debug this further, I would suggest to recompile Git with something like

static inline HANDLE help_create_file(LPCTSTR lpFileName, DWORD dwDesiredAccess,
        DWORD dwShareMode, LPSECURITY_ATTRIBUTES lpSecurityAttributes,
        DWORD dwCreationDisposition, DWORD dwFlagsAndAttributes, HANDLE hTemplateFile)
{
    return CreateFile(lpFileName, dwDesiredAccess, dwShareMode, lpSecurityAttributes,
        dwCreationDisposition, dwFlagsAndAttributes, hTemplateFile);
}
#define CreateFile(a, b, c, d, e, f, g, h) (if (ends_with(a, "01")) \
       fprintf(stderr, "%s:%d: CreateFile(%s)\n", __FILE__, __LINE__, a), \
    helper_create_file(a, b, c, d, e, f, g, h))

to figure out where the handle is opened. Then either the code itself, or starting it in gdb and running up to that call and inspecting the stack trace and the variables should give us a much better clue where the directory handle is still held.

BTW I could imagine that the directory handle is actually opened implicitly by opendir() instead. The analysis I outlined above should also work for opendir() instead of CreateFile().

@dscho dscho added the bug label Jul 8, 2015
@dscho
Copy link
Member

dscho commented Aug 12, 2015

@kostix IIRC there was a fix for this bug in 2.4.6. Could you verify, or reject, my claim that this bug is fixed?

@kostix
Copy link
Author

kostix commented Aug 12, 2015

@dscho just tested with 2.4.6-RC5 32-bit on a live repo, and no, it fails in the same way.

If you will be able to do something about this, I could prepare an "unpacked" live repository for you on the test box (I remember one can do certain tricks with git unpack-objects to get rid of pack files. And in the meantime I've added more disk space to the test box so it now has some 8GiB free).

@dscho
Copy link
Member

dscho commented Aug 13, 2015

@kostix I was wrong, the patch has not been applied yet. I talk about http://article.gmane.org/gmane.comp.version-control.msysgit/21791.

Could you (re-)build Git for Windows by applying this patch? (I typically use msysGit's https://github.com/msysgit/msysgit/blob/master/bin/apply-from-gmane.sh to apply such patches by running apply-from-gmane.sh http://article.gmane.org/gmane.comp.version-control.msysgit/21791 in /usr/src/git/).

@dscho
Copy link
Member

dscho commented Aug 13, 2015

@kostix to make it easier to test, I just pushed to the issue-231 branch, so you should be able to test via

cd /usr/src/git
git fetch origin
git checkout -t origin/issue-231
make -j15 install
cd /your/repository
git gc --aggressive

@dscho
Copy link
Member

dscho commented Aug 13, 2015

I could prepare an "unpacked" live repository for you on the test box

That would also work, just let me know whether it is easier for you to set that up, or whether it is easier to test Git built from issue-231.

@dscho
Copy link
Member

dscho commented Aug 17, 2015

As I am going through these issues to prepare for the release, I will just assume that this works as expected and merge it. (@kostix feel free to test nevertheless 😃)

dscho added a commit that referenced this issue Aug 17, 2015
This branch hopefully addresses

	#231

where `git repack` could not delete the object directory because there
was still an open handle on it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@kostix
Copy link
Author

kostix commented Aug 17, 2015

@dscho, I'm actually willing to look at it, but I just have not enough time budget for it :-(

@dscho
Copy link
Member

dscho commented Aug 17, 2015

@kostix I understand, of course. I would still feel much better about this ticket if you could verify that the bug is fixed, when you have time.

@dscho
Copy link
Member

dscho commented Aug 28, 2015

@kostix just a gentle nudge... Did you find time to test it with 2.5.0 yet?

dscho added a commit to dscho/git that referenced this issue Aug 30, 2015
This branch hopefully addresses

	git-for-windows#231

where `git repack` could not delete the object directory because there
was still an open handle on it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member

dscho commented Sep 6, 2015

@kostix Did you find time to test it with 2.5.1 yet?

dscho added a commit that referenced this issue Sep 10, 2015
This branch hopefully addresses

	#231

where `git repack` could not delete the object directory because there
was still an open handle on it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member

dscho commented Sep 10, 2015

@kostix could you please test with 2.5.2?

@kostix
Copy link
Author

kostix commented Sep 14, 2015

@dscho, 2.5.2 release 2 works for me just OK on 32-bit Windows XP SP3.

I've run git gc --aggressive on a moderately-sized repository with ca. 900 loose objects -- they have all got packed, object directories were deleted without errors, and I've verified no empty directories have been left.

Thanks!

@dscho
Copy link
Member

dscho commented Sep 14, 2015

Awesome! (And yes, you beat me: I would have asked you to test with 2.5.2(2) today 😜)

@dscho dscho closed this as completed Sep 14, 2015
dscho added a commit to dscho/git that referenced this issue Sep 18, 2015
This branch hopefully addresses

	git-for-windows#231

where `git repack` could not delete the object directory because there
was still an open handle on it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit to dscho/git that referenced this issue Sep 29, 2015
This branch hopefully addresses

	git-for-windows#231

where `git repack` could not delete the object directory because there
was still an open handle on it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit that referenced this issue Oct 5, 2015
This branch hopefully addresses

	#231

where `git repack` could not delete the object directory because there
was still an open handle on it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit to dscho/git that referenced this issue Oct 18, 2015
This branch hopefully addresses

	git-for-windows#231

where `git repack` could not delete the object directory because there
was still an open handle on it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit to dscho/git that referenced this issue Nov 7, 2015
This branch hopefully addresses

	git-for-windows#231

where `git repack` could not delete the object directory because there
was still an open handle on it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit that referenced this issue Dec 11, 2015
This branch hopefully addresses

	#231

where `git repack` could not delete the object directory because there
was still an open handle on it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
garimasi514 pushed a commit to garimasi514/git that referenced this issue Jan 6, 2020
…ing temp files

When we create temp files for downloading packs, we use a name
based on the current timestamp. There is no randomness in the
name, so we can have collisions in the same second.

Retry the temp pack names using a new "-<retry>" suffix to the
name before the ".temp".

This is a follow-up to git-for-windows#229.
jeffhostetler pushed a commit to jeffhostetler/git that referenced this issue Apr 11, 2020
…ing temp files

When we create temp files for downloading packs, we use a name
based on the current timestamp. There is no randomness in the
name, so we can have collisions in the same second.

Retry the temp pack names using a new "-<retry>" suffix to the
name before the ".temp".

This is a follow-up to git-for-windows#229.
jeffhostetler pushed a commit to jeffhostetler/git that referenced this issue Apr 13, 2020
…ing temp files

When we create temp files for downloading packs, we use a name
based on the current timestamp. There is no randomness in the
name, so we can have collisions in the same second.

Retry the temp pack names using a new "-<retry>" suffix to the
name before the ".temp".

This is a follow-up to git-for-windows#229.
jeffhostetler pushed a commit to jeffhostetler/git that referenced this issue Jun 3, 2020
Includes these pull requests:

 git-for-windows#227
 git-for-windows#228
 git-for-windows#229
 git-for-windows#231
 git-for-windows#240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
jeffhostetler pushed a commit to jeffhostetler/git that referenced this issue May 14, 2021
Includes these pull requests:

 git-for-windows#227
 git-for-windows#228
 git-for-windows#229
 git-for-windows#231
 git-for-windows#240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
jeffhostetler pushed a commit to jeffhostetler/git that referenced this issue Jun 21, 2021
Includes these pull requests:

 git-for-windows#227
 git-for-windows#228
 git-for-windows#229
 git-for-windows#231
 git-for-windows#240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
jeffhostetler pushed a commit to jeffhostetler/git that referenced this issue Aug 18, 2021
Includes these pull requests:

 git-for-windows#227
 git-for-windows#228
 git-for-windows#229
 git-for-windows#231
 git-for-windows#240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
mjcheetham pushed a commit to mjcheetham/git that referenced this issue Jun 16, 2022
Includes these pull requests:

 git-for-windows#227
 git-for-windows#228
 git-for-windows#229
 git-for-windows#231
 git-for-windows#240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
mjcheetham pushed a commit to mjcheetham/git that referenced this issue Jul 23, 2024
Includes these pull requests:

 git-for-windows#227
 git-for-windows#228
 git-for-windows#229
 git-for-windows#231
 git-for-windows#240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants