All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 4/5] safe_create_leading_directories(): fix a mkdir/rmdir race
Date: Thu, 26 Dec 2013 15:02:04 -0800	[thread overview]
Message-ID: <20131226230204.GZ20443@google.com> (raw)
In-Reply-To: <1387696451-32224-5-git-send-email-mhagger@alum.mit.edu>

Hi,

Michael Haggerty wrote:

> 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

  parent reply	other threads:[~2013-12-26 23:02 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-22  7:14 [PATCH 0/5] Fix two mkdir/rmdir races Michael Haggerty
2013-12-22  7:14 ` [PATCH 1/5] safe_create_leading_directories(): modernize format of "if" chaining Michael Haggerty
2013-12-26 21:52   ` Jonathan Nieder
2013-12-22  7:14 ` [PATCH 2/5] safe_create_leading_directories(): reduce scope of local variable Michael Haggerty
2013-12-26 21:55   ` Jonathan Nieder
2014-01-01 12:39     ` Michael Haggerty
2013-12-22  7:14 ` [PATCH 3/5] safe_create_leading_directories(): add "slash" pointer Michael Haggerty
2013-12-26 22:34   ` Jonathan Nieder
2014-01-01 21:10     ` Michael Haggerty
2013-12-22  7:14 ` [PATCH 4/5] safe_create_leading_directories(): fix a mkdir/rmdir race Michael Haggerty
2013-12-22 22:42   ` Ramsay Jones
2013-12-26 23:02   ` Jonathan Nieder [this message]
2014-01-02  0:53     ` Michael Haggerty
2013-12-22  7:14 ` [PATCH 5/5] rename_ref(): fix a mkdir()/rmdir() race Michael Haggerty
2013-12-26 23:20   ` Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131226230204.GZ20443@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.