Subject: Re: [PATCH 2/2] object-file: retry linking file into place when occluding file vanishes

From: Junio C Hamano <[email-masked]>
Date: Fri, 03 Jan 2025 08:18:12 -0800

Patrick Steinhardt <[email-masked]> 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 <[email-masked]>
> Signed-off-by: Patrick Steinhardt <[email-masked]>
> ---
>  object-file.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)

Will queue.