{"thread":{"id":"44860","subject":"git fast-import crashing on big imports","startedAt":"2017-01-12T08:30:58Z","lastAt":"2017-01-19T19:16:50Z","messageCount":9,"participants":["Ulrich Spörlein","Ulrich Spoerlein","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"309260","messageId":"20170112082138.GJ4426@acme.spoerlein.net","threadId":"44860","inReplyTo":null,"subject":"git fast-import crashing on big imports","fromName":"Ulrich Spörlein","fromEmail":"uqs@freebsd.org","sentAt":"2017-01-12T08:21:38Z","receivedAt":"2017-01-12T08:30:58Z","isPatch":false,"sender":{"key":"uqs@freebsd.org","avatar":null},"body":"Hey,\n\nthe FreeBSD svn2git conversion is crashing somewhat\nnon-deterministically during its long conversion process. From memory,\nthis was not as bad is it is with more recent versions of git (but I\ncan't be sure, really).\n\nI have a dump file that you can grab at\nhttp://scan.spoerlein.net/pub/freebsd-base.dump.xz (19G, 55G uncompressed)\nthat shows this problem after a couple of minutes of runtime. The caveat is\nthat for another member of the team on a different machine the crashes are on\ndifferent revisions.\n\nGoogling around I found two previous threads that were discussing\nproblems just like this (memory corruption, bad caching, etc)\n\nhttps://www.spinics.net/lists/git/msg93598.html  from 2009\nand\nhttp://git.661346.n2.nabble.com/long-fast-import-errors-out-quot-failed-to-apply-delta-quot-td6557884.html\nfrom 2011\n\n% git fast-import --stats < ../freebsd-base.dump\n...\nprogress SVN r49318 branch master = :49869\nprogress SVN r49319 branch stable/3 = :49870\nprogress SVN r49320 branch master = :49871\nerror: failed to apply delta\nerror: bad offset for revindex\nerror: bad offset for revindex\nerror: bad offset for revindex\nerror: bad offset for revindex\nerror: bad offset for revindex\nfatal: Can't load tree b35ae4e9c2a41677e84a3f14bed09f584c3ff25e\nfast-import: dumping crash report to fast_import_crash_29613\n\n\nfast-import crash report:\n    fast-import process: 29613\n    parent process     : 29612\n    at 2017-01-11 19:33:37 +0000\n\nfatal: Can't load tree b35ae4e9c2a41677e84a3f14bed09f584c3ff25e\n\n\ngit fsck shows a somewhat incomplete pack file (I guess that's expected if the\nprocess dies mid-stream?)\n\n% git fsck\nChecking object directories: 100% (256/256), done.\nerror: failed to apply delta6/614500)\nerror: cannot unpack d1d7ee1f81c6767c5e0f75d14d400d7512a85a0f from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack at offset 122654805\nerror: failed to apply delta\nerror: failed to read delta base object d1d7ee1f81c6767c5e0f75d14d400d7512a85a0f at offset 122654805 from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack\nerror: cannot unpack 8523bde63ef34bef725347994fdaec996d756510 from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack at offset 122671596\nerror: failed to apply delta0/614500)\nerror: failed to read delta base object d1d7ee1f81c6767c5e0f75d14d400d7512a85a0f at offset 122654805 from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack\n...\n\n\nAny comments on whether the original problems from 2009 and 2011 were ever\nfixed and committed?\n\nSome more facts:\n- git version 2.11.0\n- I don't recall these sorts of crashes with a git from 2-3 years ago\n- adding more checkpoints does help, but not fix the problem, it merely shifts\n  the crashes around to different revisions\n- incremental runs of the conversion *will* complete most of the time, but\n  depending on how often checkpoints are used, I've seen it croak on specific\n  commits and not being able to progress further :(\n\nThanks for any pointers or things to try!\nCheers\nUli\n"},{"id":"309627","messageId":"20170118140117.GK4426@acme.spoerlein.net","threadId":"44860","inReplyTo":"20170112082138.GJ4426@acme.spoerlein.net","subject":"Re: git fast-import crashing on big imports","fromName":"Ulrich Spoerlein","fromEmail":"uqs@freebsd.org","sentAt":"2017-01-18T14:01:17Z","receivedAt":"2017-01-18T14:31:38Z","isPatch":false,"sender":{"key":"uqs@freebsd.org","avatar":null},"body":"Yo Jeff, your commit 8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c from Aug\n22nd, that changes delta_base_cache to use hashmap.h is the culprit for\ngit fast-import crashing on large imports.\n\nPlease read below, you can find a 55G SVN dump that should show the\nproblem after a couple of minutes to less than an hour. Please also see\nsimilar issues from 2009 and 2011. This seems to be a rather fragile\npart of the code, could you add unit tests that make sure this\nregression is not re-introduce again once you fix it? Thanks!\n\nI'm happy to test any patches that you can provide.\n\nCheers,\nUli\n\nOn Do., 2017-01-12 at 09:21:38 +0100, Ulrich Spörlein wrote:\n> Hey,\n> \n> the FreeBSD svn2git conversion is crashing somewhat\n> non-deterministically during its long conversion process. From memory,\n> this was not as bad is it is with more recent versions of git (but I\n> can't be sure, really).\n> \n> I have a dump file that you can grab at\n> http://scan.spoerlein.net/pub/freebsd-base.dump.xz (19G, 55G uncompressed)\n> that shows this problem after a couple of minutes of runtime. The caveat is\n> that for another member of the team on a different machine the crashes are on\n> different revisions.\n> \n> Googling around I found two previous threads that were discussing\n> problems just like this (memory corruption, bad caching, etc)\n> \n> https://www.spinics.net/lists/git/msg93598.html  from 2009\n> and\n> http://git.661346.n2.nabble.com/long-fast-import-errors-out-quot-failed-to-apply-delta-quot-td6557884.html\n> from 2011\n> \n> % git fast-import --stats < ../freebsd-base.dump\n> ...\n> progress SVN r49318 branch master = :49869\n> progress SVN r49319 branch stable/3 = :49870\n> progress SVN r49320 branch master = :49871\n> error: failed to apply delta\n> error: bad offset for revindex\n> error: bad offset for revindex\n> error: bad offset for revindex\n> error: bad offset for revindex\n> error: bad offset for revindex\n> fatal: Can't load tree b35ae4e9c2a41677e84a3f14bed09f584c3ff25e\n> fast-import: dumping crash report to fast_import_crash_29613\n> \n> \n> fast-import crash report:\n>     fast-import process: 29613\n>     parent process     : 29612\n>     at 2017-01-11 19:33:37 +0000\n> \n> fatal: Can't load tree b35ae4e9c2a41677e84a3f14bed09f584c3ff25e\n> \n> \n> git fsck shows a somewhat incomplete pack file (I guess that's expected if the\n> process dies mid-stream?)\n> \n> % git fsck\n> Checking object directories: 100% (256/256), done.\n> error: failed to apply delta6/614500)\n> error: cannot unpack d1d7ee1f81c6767c5e0f75d14d400d7512a85a0f from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack at offset 122654805\n> error: failed to apply delta\n> error: failed to read delta base object d1d7ee1f81c6767c5e0f75d14d400d7512a85a0f at offset 122654805 from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack\n> error: cannot unpack 8523bde63ef34bef725347994fdaec996d756510 from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack at offset 122671596\n> error: failed to apply delta0/614500)\n> error: failed to read delta base object d1d7ee1f81c6767c5e0f75d14d400d7512a85a0f at offset 122654805 from ./objects/pack/pack-e28fcea43fc221d2ebe92857b484da58bb888237.pack\n> ...\n> \n> \n> Any comments on whether the original problems from 2009 and 2011 were ever\n> fixed and committed?\n> \n> Some more facts:\n> - git version 2.11.0\n> - I don't recall these sorts of crashes with a git from 2-3 years ago\n> - adding more checkpoints does help, but not fix the problem, it merely shifts\n>   the crashes around to different revisions\n> - incremental runs of the conversion *will* complete most of the time, but\n>   depending on how often checkpoints are used, I've seen it croak on specific\n>   commits and not being able to progress further :(\n> \n> Thanks for any pointers or things to try!\n> Cheers\n> Uli\n"},{"id":"309628","messageId":"20170118143814.or34vxxwjwnzg5jz@sigill.intra.peff.net","threadId":"44860","inReplyTo":"20170118140117.GK4426@acme.spoerlein.net","subject":"Re: git fast-import crashing on big imports","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-18T14:38:14Z","receivedAt":"2017-01-18T14:38:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 18, 2017 at 03:01:17PM +0100, Ulrich Spoerlein wrote:\n\n> Yo Jeff, your commit 8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c from Aug\n> 22nd, that changes delta_base_cache to use hashmap.h is the culprit for\n> git fast-import crashing on large imports.\n\nI actually saw your bug report the other day and tried to download the\ndump file, but got a 404. Can you double check that it is available?\n\n-Peff\n"},{"id":"309661","messageId":"20170118200646.6larm2qu32xm73on@sigill.intra.peff.net","threadId":"44860","inReplyTo":"20170118143814.or34vxxwjwnzg5jz@sigill.intra.peff.net","subject":"Re: git fast-import crashing on big imports","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-18T20:06:46Z","receivedAt":"2017-01-18T20:33:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 18, 2017 at 09:38:14AM -0500, Jeff King wrote:\n\n> On Wed, Jan 18, 2017 at 03:01:17PM +0100, Ulrich Spoerlein wrote:\n> \n> > Yo Jeff, your commit 8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c from Aug\n> > 22nd, that changes delta_base_cache to use hashmap.h is the culprit for\n> > git fast-import crashing on large imports.\n> \n> I actually saw your bug report the other day and tried to download the\n> dump file, but got a 404. Can you double check that it is available?\n\nThe URL in your original mail was bogus:\n\n> > I have a dump file that you can grab at\n> > http://scan.spoerlein.net/pub/freebsd-base.dump.xz (19G, 55G uncompressed)\n\nVia guess-and-check, I found:\n\n  http://www.spoerlein.net/pub/freebsd-base.git.fi.xz\n\nI'll see if I can reproduce the problem.\n\n-Peff\n"},{"id":"309662","messageId":"20170118202704.w6pjxfvnge7utk34@sigill.intra.peff.net","threadId":"44860","inReplyTo":"CAJ9axoSzZJXD4RKvVx+D60dw4sakMJWgNmOP-cREWA53Ae3C3w@mail.gmail.com","subject":"Re: git fast-import crashing on big imports","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-18T20:27:04Z","receivedAt":"2017-01-18T20:33:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 18, 2017 at 09:20:07PM +0100, Ulrich Spörlein wrote:\n\n> I found your commit via bisect in case you were wondering. Thanks for\n> picking this up.\n\nStill downloading. However, just looking at the code, the obvious\nculprit would be clear_delta_base_cache(), which is called from\nliterally nowhere except fast-import, and then only when checkpointing.\n\n-Peff\n"},{"id":"309679","messageId":"20170118215120.6hle2uxgkcvvtlox@sigill.intra.peff.net","threadId":"44860","inReplyTo":"20170118202704.w6pjxfvnge7utk34@sigill.intra.peff.net","subject":"Re: git fast-import crashing on big imports","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-18T21:51:20Z","receivedAt":"2017-01-18T21:51:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 18, 2017 at 03:27:04PM -0500, Jeff King wrote:\n\n> On Wed, Jan 18, 2017 at 09:20:07PM +0100, Ulrich Spörlein wrote:\n> \n> > I found your commit via bisect in case you were wondering. Thanks for\n> > picking this up.\n> \n> Still downloading. However, just looking at the code, the obvious\n> culprit would be clear_delta_base_cache(), which is called from\n> literally nowhere except fast-import, and then only when checkpointing.\n\nHmm. I haven't reproduced your exact issue, but I was able to produce\nsome hijinks in that function.\n\nThe problem is that the hashmap_iter interface is unreliable if entries\nare added or removed from the map during the iteration.\n\nI suspect the patch below may fix things for you. It works around it by\nwalking over the lru list (either is fine, as they both contain all\nentries, and since we're clearing everything, we don't care about the\norder).\n\n---\n sha1_file.c | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 1eb47f611..d20714d6b 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2342,11 +2342,10 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)\n \n void clear_delta_base_cache(void)\n {\n-\tstruct hashmap_iter iter;\n-\tstruct delta_base_cache_entry *entry;\n-\tfor (entry = hashmap_iter_first(&delta_base_cache, &iter);\n-\t     entry;\n-\t     entry = hashmap_iter_next(&iter)) {\n+\tstruct list_head *lru, *tmp;\n+\tlist_for_each_safe(lru, tmp, &delta_base_cache_lru) {\n+\t\tstruct delta_base_cache_entry *entry =\n+\t\t\tlist_entry(lru, struct delta_base_cache_entry, lru);\n \t\trelease_delta_base_cache(entry);\n \t}\n }\n-- \n2.11.0.698.gd6b48ab4c\n\n\n\n\n> \n> -Peff\n"},{"id":"309733","messageId":"20170119163350.zsfb33lmigkyljjh@sigill.intra.peff.net","threadId":"44860","inReplyTo":"CAJ9axoT1pUQc_jTKxO+RMw7emhA4ss1NCAU+hpnyG5LMwGD89w@mail.gmail.com","subject":"[PATCH] clear_delta_base_cache(): don't modify hashmap while iterating","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-19T16:33:50Z","receivedAt":"2017-01-19T16:34:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 19, 2017 at 03:03:46PM +0100, Ulrich Spörlein wrote:\n\n> > I suspect the patch below may fix things for you. It works around it by\n> > walking over the lru list (either is fine, as they both contain all\n> > entries, and since we're clearing everything, we don't care about the\n> > order).\n> \n> Confirmed. With the patch applied, I can import the whole 55G in one go\n> without any crashes or aborts. Thanks much!\n\nThanks. Here it is rolled up with a commit message.\n\n-- >8 --\nSubject: clear_delta_base_cache(): don't modify hashmap while iterating\n\nRemoving entries while iterating causes fast-import to\naccess an already-freed `struct packed_git`, leading to\nvarious confusing errors.\n\nWhat happens is that clear_delta_base_cache() drops the\nwhole contents of the cache by iterating over the hashmap,\ncalling release_delta_base_cache() on each entry. That\nfunction removes the item from the hashmap. The hashmap code\nmay then shrink the table, but the hashmap_iter struct\nretains an offset from the old table.\n\nAs a result, the next call to hashmap_iter_next() may claim\nthat the iteration is done, even though some items haven't\nbeen visited.\n\nThe only caller of clear_delta_base_cache() is fast-import,\nwhich wants to clear the cache because it is discarding the\npacked_git struct for its temporary pack. So by failing to\nremove all of the entries, we still have references to the\nfreed packed_git.\n\nTo make things even more confusing, this doesn't seem to\ntrigger with the test suite, because it depends on\ncomplexities like the size of the hash table, which entries\ngot cleared, whether we try to access them before they're\nevicted from the cache, etc.\n\nSo I've been able to identify the problem with large\nimports like freebsd's svn import, or a fast-export of\nlinux.git. But nothing that would be reasonable to run as\npart of the normal test suite.\n\nWe can fix this easily by iterating over the lru linked list\ninstead of the hashmap. They both contain the same entries,\nand we can use the \"safe\" variant of the list iterator,\nwhich exists for exactly this case.\n\nLet's also add a warning to the hashmap API documentation to\nreduce the chances of getting bit by this again.\n\nReported-by: Ulrich Spörlein <uqs@freebsd.org>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/technical/api-hashmap.txt | 4 +++-\n sha1_file.c                             | 9 ++++-----\n 2 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/technical/api-hashmap.txt b/Documentation/technical/api-hashmap.txt\nindex 28f5a8b71..a3f020cd9 100644\n--- a/Documentation/technical/api-hashmap.txt\n+++ b/Documentation/technical/api-hashmap.txt\n@@ -188,7 +188,9 @@ Returns the removed entry, or NULL if not found.\n `void *hashmap_iter_next(struct hashmap_iter *iter)`::\n `void *hashmap_iter_first(struct hashmap *map, struct hashmap_iter *iter)`::\n \n-\tUsed to iterate over all entries of a hashmap.\n+\tUsed to iterate over all entries of a hashmap. Note that it is\n+\tnot safe to add or remove entries to the hashmap while\n+\titerating.\n +\n `hashmap_iter_init` initializes a `hashmap_iter` structure.\n +\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 1eb47f611..d20714d6b 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2342,11 +2342,10 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)\n \n void clear_delta_base_cache(void)\n {\n-\tstruct hashmap_iter iter;\n-\tstruct delta_base_cache_entry *entry;\n-\tfor (entry = hashmap_iter_first(&delta_base_cache, &iter);\n-\t     entry;\n-\t     entry = hashmap_iter_next(&iter)) {\n+\tstruct list_head *lru, *tmp;\n+\tlist_for_each_safe(lru, tmp, &delta_base_cache_lru) {\n+\t\tstruct delta_base_cache_entry *entry =\n+\t\t\tlist_entry(lru, struct delta_base_cache_entry, lru);\n \t\trelease_delta_base_cache(entry);\n \t}\n }\n-- \n2.11.0.747.g2c5918130\n\n"},{"id":"309740","messageId":"CAJ9axoT1pUQc_jTKxO+RMw7emhA4ss1NCAU+hpnyG5LMwGD89w@mail.gmail.com","threadId":"44860","inReplyTo":"20170118215120.6hle2uxgkcvvtlox@sigill.intra.peff.net","subject":"Re: git fast-import crashing on big imports","fromName":"Ulrich Spörlein","fromEmail":"uqs@freebsd.org","sentAt":"2017-01-19T14:03:46Z","receivedAt":"2017-01-19T17:32:39Z","isPatch":false,"sender":{"key":"uqs@freebsd.org","avatar":null},"body":"2017-01-18 22:51 GMT+01:00 Jeff King <peff@peff.net>:\n>\n> On Wed, Jan 18, 2017 at 03:27:04PM -0500, Jeff King wrote:\n>\n> > On Wed, Jan 18, 2017 at 09:20:07PM +0100, Ulrich Spörlein wrote:\n> >\n> > > I found your commit via bisect in case you were wondering. Thanks for\n> > > picking this up.\n> >\n> > Still downloading. However, just looking at the code, the obvious\n> > culprit would be clear_delta_base_cache(), which is called from\n> > literally nowhere except fast-import, and then only when checkpointing.\n\nSorry for the bad URL, pesky last minute changes ...\n\n> Hmm. I haven't reproduced your exact issue, but I was able to produce\n> some hijinks in that function.\n>\n> The problem is that the hashmap_iter interface is unreliable if entries\n> are added or removed from the map during the iteration.\n>\n> I suspect the patch below may fix things for you. It works around it by\n> walking over the lru list (either is fine, as they both contain all\n> entries, and since we're clearing everything, we don't care about the\n> order).\n\nConfirmed. With the patch applied, I can import the whole 55G in one go\nwithout any crashes or aborts. Thanks much!\n\n>\n> ---\n>  sha1_file.c | 9 ++++-----\n>  1 file changed, 4 insertions(+), 5 deletions(-)\n>\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 1eb47f611..d20714d6b 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -2342,11 +2342,10 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)\n>\n>  void clear_delta_base_cache(void)\n>  {\n> -       struct hashmap_iter iter;\n> -       struct delta_base_cache_entry *entry;\n> -       for (entry = hashmap_iter_first(&delta_base_cache, &iter);\n> -            entry;\n> -            entry = hashmap_iter_next(&iter)) {\n> +       struct list_head *lru, *tmp;\n> +       list_for_each_safe(lru, tmp, &delta_base_cache_lru) {\n> +               struct delta_base_cache_entry *entry =\n> +                       list_entry(lru, struct delta_base_cache_entry, lru);\n>                 release_delta_base_cache(entry);\n>         }\n>  }\n> --\n> 2.11.0.698.gd6b48ab4c\n>\n>\n>\n>\n> >\n> > -Peff\n"},{"id":"309757","messageId":"xmqqh94u2739.fsf@gitster.mtv.corp.google.com","threadId":"44860","inReplyTo":"20170119163350.zsfb33lmigkyljjh@sigill.intra.peff.net","subject":"Re: [PATCH] clear_delta_base_cache(): don't modify hashmap while iterating","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-19T19:16:42Z","receivedAt":"2017-01-19T19:16:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Thanks. Here it is rolled up with a commit message.\n>\n> -- >8 --\n> Subject: clear_delta_base_cache(): don't modify hashmap while iterating\n>\n> Removing entries while iterating causes fast-import to\n> access an already-freed `struct packed_git`, leading to\n> various confusing errors.\n>\n> What happens is that clear_delta_base_cache() drops the\n> whole contents of the cache by iterating over the hashmap,\n> calling release_delta_base_cache() on each entry. That\n> function removes the item from the hashmap. The hashmap code\n> may then shrink the table, but the hashmap_iter struct\n> retains an offset from the old table.\n>\n> As a result, the next call to hashmap_iter_next() may claim\n> that the iteration is done, even though some items haven't\n> been visited.\n>\n> The only caller of clear_delta_base_cache() is fast-import,\n> which wants to clear the cache because it is discarding the\n> packed_git struct for its temporary pack. So by failing to\n> remove all of the entries, we still have references to the\n> freed packed_git.\n>\n> To make things even more confusing, this doesn't seem to\n> trigger with the test suite, because it depends on\n> complexities like the size of the hash table, which entries\n> got cleared, whether we try to access them before they're\n> evicted from the cache, etc.\n>\n> So I've been able to identify the problem with large\n> imports like freebsd's svn import, or a fast-export of\n> linux.git. But nothing that would be reasonable to run as\n> part of the normal test suite.\n>\n> We can fix this easily by iterating over the lru linked list\n> instead of the hashmap. They both contain the same entries,\n> and we can use the \"safe\" variant of the list iterator,\n> which exists for exactly this case.\n>\n> Let's also add a warning to the hashmap API documentation to\n> reduce the chances of getting bit by this again.\n>\n> Reported-by: Ulrich Spörlein <uqs@freebsd.org>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n\nMakes sense.  Thanks, both, for reporting, finding and fixing.\n\nWill apply.\n\n>  Documentation/technical/api-hashmap.txt | 4 +++-\n>  sha1_file.c                             | 9 ++++-----\n>  2 files changed, 7 insertions(+), 6 deletions(-)\n>\n> diff --git a/Documentation/technical/api-hashmap.txt b/Documentation/technical/api-hashmap.txt\n> index 28f5a8b71..a3f020cd9 100644\n> --- a/Documentation/technical/api-hashmap.txt\n> +++ b/Documentation/technical/api-hashmap.txt\n> @@ -188,7 +188,9 @@ Returns the removed entry, or NULL if not found.\n>  `void *hashmap_iter_next(struct hashmap_iter *iter)`::\n>  `void *hashmap_iter_first(struct hashmap *map, struct hashmap_iter *iter)`::\n>  \n> -\tUsed to iterate over all entries of a hashmap.\n> +\tUsed to iterate over all entries of a hashmap. Note that it is\n> +\tnot safe to add or remove entries to the hashmap while\n> +\titerating.\n>  +\n>  `hashmap_iter_init` initializes a `hashmap_iter` structure.\n>  +\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 1eb47f611..d20714d6b 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -2342,11 +2342,10 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)\n>  \n>  void clear_delta_base_cache(void)\n>  {\n> -\tstruct hashmap_iter iter;\n> -\tstruct delta_base_cache_entry *entry;\n> -\tfor (entry = hashmap_iter_first(&delta_base_cache, &iter);\n> -\t     entry;\n> -\t     entry = hashmap_iter_next(&iter)) {\n> +\tstruct list_head *lru, *tmp;\n> +\tlist_for_each_safe(lru, tmp, &delta_base_cache_lru) {\n> +\t\tstruct delta_base_cache_entry *entry =\n> +\t\t\tlist_entry(lru, struct delta_base_cache_entry, lru);\n>  \t\trelease_delta_base_cache(entry);\n>  \t}\n>  }\n"}]}