From: Junio C Hamano Date: Wed, 16 Jan 2008 01:27:49 GMT Subject: Re: git-commit fatal: Out of memory? mmap failed: Bad file descriptor Message-ID: <7vmyr6bluy.fsf@gitster.siamese.dyndns.org> In-Reply-To: <7vzlv6d6sa.fsf@gitster.siamese.dyndns.org> Junio C Hamano writes: > Linus Torvalds writes: > >> 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... I'll aplly and push out Kristian's one that apparently got Tested-by from Brandon for tonight.