mirror of
https://github.com/git/git.git
synced 2024-11-25 19:04:18 +08:00
abd5a00268
On Thu, Jan 19, 2017 at 03:03:46PM +0100, Ulrich Spörlein wrote: > > I suspect the patch below may fix things for you. It works around it by > > walking over the lru list (either is fine, as they both contain all > > entries, and since we're clearing everything, we don't care about the > > order). > > Confirmed. With the patch applied, I can import the whole 55G in one go > without any crashes or aborts. Thanks much! Thanks. Here it is rolled up with a commit message. -- >8 -- Subject: clear_delta_base_cache(): don't modify hashmap while iterating Removing entries while iterating causes fast-import to access an already-freed `struct packed_git`, leading to various confusing errors. What happens is that clear_delta_base_cache() drops the whole contents of the cache by iterating over the hashmap, calling release_delta_base_cache() on each entry. That function removes the item from the hashmap. The hashmap code may then shrink the table, but the hashmap_iter struct retains an offset from the old table. As a result, the next call to hashmap_iter_next() may claim that the iteration is done, even though some items haven't been visited. The only caller of clear_delta_base_cache() is fast-import, which wants to clear the cache because it is discarding the packed_git struct for its temporary pack. So by failing to remove all of the entries, we still have references to the freed packed_git. To make things even more confusing, this doesn't seem to trigger with the test suite, because it depends on complexities like the size of the hash table, which entries got cleared, whether we try to access them before they're evicted from the cache, etc. So I've been able to identify the problem with large imports like freebsd's svn import, or a fast-export of linux.git. But nothing that would be reasonable to run as part of the normal test suite. We can fix this easily by iterating over the lru linked list instead of the hashmap. They both contain the same entries, and we can use the "safe" variant of the list iterator, which exists for exactly this case. Let's also add a warning to the hashmap API documentation to reduce the chances of getting bit by this again. Reported-by: Ulrich Spörlein <uqs@freebsd.org> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> |
||
|---|---|---|
| .. | ||
| .gitignore | ||
| api-allocation-growing.txt | ||
| api-argv-array.txt | ||
| api-builtin.txt | ||
| api-config.txt | ||
| api-credentials.txt | ||
| api-decorate.txt | ||
| api-diff.txt | ||
| api-directory-listing.txt | ||
| api-error-handling.txt | ||
| api-gitattributes.txt | ||
| api-grep.txt | ||
| api-hashmap.txt | ||
| api-history-graph.txt | ||
| api-in-core-index.txt | ||
| api-index-skel.txt | ||
| api-index.sh | ||
| api-merge.txt | ||
| api-object-access.txt | ||
| api-parse-options.txt | ||
| api-quote.txt | ||
| api-ref-iteration.txt | ||
| api-remote.txt | ||
| api-revision-walking.txt | ||
| api-run-command.txt | ||
| api-setup.txt | ||
| api-sha1-array.txt | ||
| api-sigchain.txt | ||
| api-string-list.txt | ||
| api-submodule-config.txt | ||
| api-trace.txt | ||
| api-tree-walking.txt | ||
| api-xdiff-interface.txt | ||
| bitmap-format.txt | ||
| http-protocol.txt | ||
| index-format.txt | ||
| pack-format.txt | ||
| pack-heuristics.txt | ||
| pack-protocol.txt | ||
| protocol-capabilities.txt | ||
| protocol-common.txt | ||
| racy-git.txt | ||
| repository-version.txt | ||
| send-pack-pipeline.txt | ||
| shallow.txt | ||
| signature-format.txt | ||
| trivial-merge.txt | ||