git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org,  Jeff King <peff@peff.net>
Subject: Re: [PATCH 2/2] object-file: retry linking file into place when occluding file vanishes
Date: Fri, 03 Jan 2025 08:18:12 -0800	[thread overview]
Message-ID: <xmqqo70nswfv.fsf@gitster.g> (raw)
In-Reply-To: <20250103-b4-pks-object-file-racy-collision-check-v1-2-6ef9e2da1f87@pks.im> (Patrick Steinhardt's message of "Fri, 03 Jan 2025 09:19:55 +0100")

Patrick Steinhardt <ps@pks.im> writes:

> In a preceding commit, we have adapted `check_collision()` to ignore
> the case where either of the colliding files vanishes. This should be
> safe in general when we assume that the contents of these two files were
> the same. But the check is all about detecting collisions, so that
> assumption may be too optimistic.
>
> Adapt the code to retry linking the object into place when we see that
> the destination file has racily vanished. This should generally succeed
> as we have just observed that the destination file does not exist
> anymore, except in the very unlikely event that it gets recreated by
> another concurrent process again.

OK.  

The way the new code is structured, we are set to infinitely retry
as long as our link() fails with EEXIST yet check_collision() finds
that the destination is missing.  It makes me somewhat uneasy, but
such a condition needs an actively racing attacker that can perfectly
time the creation and the removal of the destination, so it would
probably be OK if this code lacks "we retry only once and fail"
safety valve.

> Furthermore, stop treating `ENOENT` specially for the source file. It
> shouldn't happen that the source vanishes as we're using a fresh
> temporary file for it, so if it does vanish it indicates an actual
> error.

Makes sense.

> Suggested-by: Jeff King <peff@peff.net>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  object-file.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)

Will queue.

  reply	other threads:[~2025-01-03 16:18 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-03  8:19 [PATCH 0/2] object-file: retry linking file into place when occluding file vanishes Patrick Steinhardt
2025-01-03  8:19 ` [PATCH 1/2] object-file: rename variables in `check_collision()` Patrick Steinhardt
2025-01-03 19:10   ` Jeff King
2025-01-03  8:19 ` [PATCH 2/2] object-file: retry linking file into place when occluding file vanishes Patrick Steinhardt
2025-01-03 16:18   ` Junio C Hamano [this message]
2025-01-03 19:40   ` Jeff King
2025-01-03 19:59     ` Jeff King
2025-01-03 22:29       ` Junio C Hamano
2025-01-06 11:11       ` Patrick Steinhardt
2025-01-07  1:25         ` Jeff King
2025-01-03 20:25     ` Junio C Hamano
2025-01-06 11:11     ` Patrick Steinhardt
2025-01-06  9:24 ` [PATCH v2 0/3] " Patrick Steinhardt
2025-01-06  9:24   ` [PATCH v2 1/3] object-file: rename variables in `check_collision()` Patrick Steinhardt
2025-01-06  9:24   ` [PATCH v2 2/3] object-file: don't special-case missing source file in collision check Patrick Steinhardt
2025-01-06  9:24   ` [PATCH v2 3/3] object-file: retry linking file into place when occluding file vanishes Patrick Steinhardt

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=xmqqo70nswfv.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).