Re: [PATCH] receive-pack: fix stale packfile locks when dying

From: Taylor Blau <me@ttaylorr.com>
Date: Mar 9, 2023, 18:26 UTC
Message-ID: <ZAokxsXGwgdNEmDc@nand.local>
In-Reply-To: <e16bd81bf9e251aa6959fbe10a3fbc215a4a1c12.1678367338.git.ps@pks.im>

On Thu, Mar 09, 2023 at 02:09:23PM +0100, Patrick Steinhardt wrote:

> Fix this race by installing an atexit(3P) handler that unlinks the keep
> file.

This reminded me of a discussion that I thought you and I had a few months ago on the list about whether or not it was safe to call unlink() in an async signal handler.

TL;DR, it is, and the link back to that discussion is here:

  https://lore.kernel.org/git/YdjBkZsnYd+zYne1@nand.local/

Show 19 quoted lines

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/receive-pack.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index cd5c7a28ef..0a6030d775 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -2186,6 +2186,12 @@ static const char *parse_pack_header(struct pack_header *hdr)
>
>  static const char *pack_lockfile;
>
> +static void unlink_pack_lockfile(void)
> +{
> +	if (pack_lockfile)
> +		unlink(pack_lockfile);
> +}
> +

...and I think that this would all work, but I agree that using the tempfile API here (as Peff suggests below) would probably be more ergonomic.

Thanks, Taylor