Re: [PATCH 4/5] safe_create_leading_directories(): fix a mkdir/rmdir race

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 26, 2013, 23:02 UTC
Message-ID: <20131226230204.GZ20443@google.com>
In-Reply-To: <1387696451-32224-5-git-send-email-mhagger@alum.mit.edu>

Hi,

Michael Haggerty wrote:

Show 7 quoted lines

> It could be that some other process is trying to clean up empty
> directories at the same time that safe_create_leading_directories() is
> attempting to create them.  In this case, it could happen that
> directory "a/b" was present at the end of one iteration of the loop
> (either it was already present or we just created it ourselves), but
> by the time we try to create directory "a/b/c", directory "a/b" has
> been deleted.  In fact, directory "a" might also have been deleted.

When does this happen in practice? Is this about git racing with itself or with some other program?

I fear that the aggressive directory creator fighting the aggressive directory remover might be waging a losing battle.

Is this about a push that creates a ref racing against a push that deletes a ref from the same hierarchy?

> So, if a call to mkdir() fails with ENOENT, then try checking/making
> all directories again from the beginning.  Attempt up to three times
> before giving up.

If we are racing against a ref deletion, then we can retry while our rival keeps walking up the directory tree and deleting parent directories. As soon as we successfully create a directory, we have won the race.

But what happens if the entire safe_create_leading_directories operation succeeds and *then* our racing partner deletes the directory? No one is putting in a file to reserve the directory for the directory creator.

If we care enough to retry more than once, I fear this is retrying at the wrong level.

> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  sha1_file.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Tests?

The code is clear and implements the design correctly.

Thanks for some food for thought, Jonathan