Re: [PATCH] Fix handle leak in builtin-pack-objects

From: Nicolas Pitre <nico@cam.org>
Date: Nov 19, 2008, 14:31 UTC
Message-ID: <alpine.LFD.2.00.0811190917020.27509@xanadu.home>
In-Reply-To: <49240F6D.3030203@viscovery.net>

On Wed, 19 Nov 2008, Johannes Sixt wrote:

Show 22 quoted lines

> Nicolas Pitre schrieb:
> > On Wed, 19 Nov 2008, Johannes Sixt wrote:
> > 
> >> Alex Riesen schrieb:
> >>> The opened packs seem to stay open forever.
> >> In my MinGW port I have the patch below that avoids that t5303 fails
> >> because of a pack file that remains open. (Open files cannot be replaced
> >> on Windows.) I had hoped that your patch would help, but it does not.
> >> Something else still keeps the pack file open. Can anything be done about
> >> that?
> >>
> >> -- Hannes
> >>
> >> From: Johannes Sixt <j6t@kdbg.org>
> >> Date: Mon, 17 Nov 2008 09:25:19 +0100
> >> Subject: [PATCH] t5303: Do not overwrite an existing pack
> >>
> ...
> > Acked-by: Nicolas Pitre <nico@cam.org>
> 
> Thanks, but I should have mentioned that at this time this patch was just
> meant for exposition, not inclusion.

Well, I'd include it right away since it is fundamentally the right thing to do.

> I'd prefer a solution to the problem that the pack file remains open. Do
> you have an idea where git-pack-objects keeps the pack file open, even
> with Alex's two patches applied?

That's not the issue.

The test is using pack-objects to overwrite a pack file, but to do so pack-objects has to open and read from the pack file about to be overwritten. This is just wrong even if it happens to work by luck on Linux.

It is on purpose that the pack files are kept open. This is to minimize the number of open() and mmap()/read() operations between successive object reads. Those are not leaked file handles since they get closed when new packs are opened and the number of concurrent opened packs reaches a certain limit.

Nicolas