From: Johannes Sixt Date: Wed, 21 Jan 2026 10:14:38 GMT Subject: Re: [PATCH v5] lockfile: add PID file for debugging stale locks Message-ID: In-Reply-To: Am 21.01.26 um 09:13 schrieb Eric Sunshine: > On Wed, Jan 21, 2026 at 2:15 AM Jeff King wrote: >> I don't think it's wrong, but the cleanup is redundant between the "out" >> path and the others. >> >> Did you mean this: >> >> diff --git a/lockfile.c b/lockfile.c >> @@ -122,14 +122,10 @@ static struct tempfile *create_lock_pid_file(const char *pid_path, int mode) >> strbuf_addf(&content, "pid %" PRIuMAX "\n", (uintmax_t)getpid()); >> if (write_in_full(fd, content.buf, content.len) < 0) { >> warning_errno(_("could not write lock pid file '%s'"), pid_path); >> - close(fd); >> - fd = -1; >> unlink(pid_path); >> goto out; >> } >> >> - close(fd); >> - fd = -1; >> pid_tempfile = register_tempfile(pid_path); >> >> out: >> >> which would just let the close after the out label handle all cases? > > Correct me if I'm wrong, but wouldn't this suggested change be > problematic on Microsoft Windows? Specifically, if I recall correctly, > Windows won't allow a file to be deleted if any processes still have > it open, and this change eliminates the call to close() preceding the > call to unlink(), so the file would still be held open when the > attempt is made to remove it. You analysis is correct. I was just about to point this out, too. -- Hannes