Race condition with git-status and git-rebase

Hi, Since I don't know how to fix it, this is my bug report: git-status reads the index file then holds the `index.lock', it writes the updated index in `index.lock' and renames the `index.lock' to `index', but if it used with git-rebase then there is a (rare) race condition. I reproduce this

Re: Race condition with git-status and git-rebase

Maybe you will find it useful: this is one way that I use to manually debug the race condition (and it works in Linux): diff --git a/builtin/commit.c b/builtin/commit.c index d9550c5..c0511f6 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1283,6 +1283,8 @@ int cmd_status(int argc, const

[PATCH] Verify index file before we opportunistically update it

to not proceed to opportunistic update. Some questions: 1) Is it better to have these functions as static? 2) If the answer of (1) is no, should I define verify_cache*() also? 3) If something goes wrong in verify_hdr(), it will print an error message, should I make a quietly version of it? Yiannis

[PATCH] Verify index file before we opportunistically update it

index 5. process A applies 2nd commit 6. process B takes the lock, then overwrites process A's changes. 7. process A applies 3rd commit As an end result the 3rd commit will have a revert of the 2nd commit. Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com --- cache.h | 3

[PATCH v2] Verify index file before we opportunistically update it

index 5. process A applies 2nd commit 6. process B takes the lock, then overwrites process A's changes. 7. process A applies 3rd commit As an end result the 3rd commit will have a revert of the 2nd commit. Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com --- cache.h | 3

[PATCH v3] Verify index file before we opportunistically update it

index 5. process A applies 2nd commit 6. process B takes the lock, then overwrites process A's changes. 7. process A applies 3rd commit As an end result the 3rd commit will have a revert of the 2nd commit. Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com --- cache.h | 3

[PATCH v4] Verify index file before we opportunistically update it

process A's changes. 7. process A applies 3rd commit As an end result the 3rd commit will have a revert of the 2nd commit. When process B takes the lock, it needs to make sure that the index hasn't changed since step 4. Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com --- Version 4

[PATCH v5] Verify index file before we opportunistically update it

process A's changes. 7. process A applies 3rd commit As an end result the 3rd commit will have a revert of the 2nd commit. When process B takes the lock, it needs to make sure that the index hasn't changed since step 4. Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com --- Version 4

Re: [PATCH v5] Verify index file before we opportunistically update it

On Thu, Apr 10, 2014 at 05:40:55PM +0700, Duy Nguyen wrote: verify_hdr() is a bit expensive because you need to digest the whole index file (could big as big as 14MB on webkit). Could we get away without it? I mean, is it enough that we pick the last 20 bytes and compare it with istate-sha1?

[PATCH v6] Verify index file before we opportunistically update it

process A's changes. 7. process A applies 3rd commit As an end result the 3rd commit will have a revert of the 2nd commit. When process B takes the lock, it needs to make sure that the index hasn't changed since step 4. Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com --- Version 4

[PATCH v7 1/2] Add xpread() and xpwrite()

xpread() and xpwrite() pay attention to EAGAIN/EINTR, so they will resume automatically on interrupted call. Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com --- builtin/index-pack.c | 2 +- compat/mmap.c| 4 +--- git-compat-util.h| 2 ++ wrapper.c| 36

[PATCH v7 2/2] Verify index file before we opportunistically update it

process A's changes. 7. process A applies 3rd commit As an end result the 3rd commit will have a revert of the 2nd commit. When process B takes the lock, it needs to make sure that the index hasn't changed since step 4. Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com --- Version 4

Re: [PATCH v7 1/2] Add xpread() and xpwrite()

On Thu, Apr 10, 2014 at 11:35:42AM -0700, Junio C Hamano wrote: We do not even use pwrite(); please don't add anything unnecessary and unexercised, like xpwrite(), as potential bugs in it will go unnoticed long after its introduction until it first gets used. Correct, my mistake. -- To

[PATCH v8 1/2] Add xpread()

xpread() pay attention to EAGAIN/EINTR, so it will resume automatically on interrupted call. Signed-off-by: Yiannis Marangos yiannis.maran...@gmail.com --- This is actually the 2nd version of this commit but before I emailed it as version 7 because it's a dependency of [PATCH v7 2/2

Re: [PATCH v7 2/2] Verify index file before we opportunistically update it

On Fri, Apr 11, 2014 at 09:47:09AM +0200, Torsten Bögershausen wrote: 6. process A applies a commit: - read the index into memory - take the lock - update the index file on disc - release the lock So here we can have race condition. Maybe we should implement Duy's

Re: [PATCH v7 2/2] Verify index file before we opportunistically update it

On Fri, Apr 11, 2014 at 01:43:47PM -0700, Junio C Hamano wrote: Having said that, nobody sane would be running two simultaneous operations that are clearly write-oriented competing with each other against the same index file. So in that sense that can be done as a less urgent follow-up for