Re: git-commit fatal: Out of memory? mmap failed: Bad file descriptor

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 15, 2008, 23:10 UTC
Message-ID: <7vzlv6d6sa.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.1.00.0801151036110.2806@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

Show 20 quoted lines

> 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.
>
> The bug really is pretty stupid:
>
>  - prepare_index() does a 
>
> 	fd = hold_lock_file_for_update(&false_lock, ...
> 	...
> 	if (write_cache(fd, active_cache, active_nr) || close(fd))
> 		die("unable to write temporary index file");
>
> and the magic here is that *it*closes*the*fd*.

While I think the ones that are immediately followed by commit_locked_index() can drop the close(fd) safely, I am not sure about Kristian's changes to the other ones that we currently close(fd) but do not commit nor rollback immediately. These indices are now shown to the hook with open fd to it if you choose not to close them. Is that okay for Windows guys? I somehow had an impression that the other process may have trouble accessing a file that is still open elsewhere for writing.

So I think the approach along the lines of your "hack" to close and tell lockfile API not to double-close is more appropriate. We would perhaps want "close_lock_file(struct lock_file *)" that calls close(lk->fd) and does lk->fd = -1 without rename/unlink, and replace these close() with that.

I am sick today, feeling feverish, and not thinking straight, so I may be talking total nonsense...