Jeff King <peff@xxxxxxxx> writes: > There's a bug in builtin/am.c in which we take a lock on > MERGE_RR recursively. But rather than fix am.c, this patch > fixes the confusing interface from rerere.c that caused the > bug. Read on for the gory details. > > The setup_rerere() function both reads the existing MERGE_RR > file, and takes MERGE_RR.lock. In the rerere() and > rerere_forget() functions, we end up in write_rr(), which > will then commit the lock file. > > But for functions like rerere_clear() that do not write to > MERGE_RR, we expect the caller to have handled > setup_rerere(). That caller would then need to release the > lockfile, but it can't; the lock struct is local to > rerere.c. > > For builtin/rerere.c, this is OK. We run a single rerere > operation and then exit immediately, which has the side > effect of rolling back the lockfile. > > But in builtin/am.c, this is actively wrong. If we run "git > am -3 --skip", we call setup-rerere twice without releasing > the lock: > > 1. The "--skip" causes us to call am_rerere_clear(), which > calls setup_rerere(), but never drops the lock. > > 2. We then proceed to the next patch. > > 3. The "--3way" may cause us to call rerere() to handle > conflicts in that patch, but we are already holding the > lock. The lockfile code dies with: > > BUG: prepare_tempfile_object called for active object Thanks. I've been re-acquainting with the internals of rerere and agree with your approach to this issue. Let's merge this before -rc1. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html