git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, Han-Wen Nienhuys <hanwenn@gmail.com>,
	Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [PATCH v2 07/11] reftable/stack: fix stale lock when dying
Date: Mon, 11 Dec 2023 10:08:48 +0100	[thread overview]
Message-ID: <ZXbRoDh7j18olBDx@tanuki> (raw)
In-Reply-To: <ZXOXpQstse8CdI7J@nand.local>

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]

On Fri, Dec 08, 2023 at 05:24:37PM -0500, Taylor Blau wrote:
> On Fri, Dec 08, 2023 at 03:53:23PM +0100, Patrick Steinhardt wrote:
> > When starting a transaction via `reftable_stack_init_addition()`, we
> > create a lockfile for the reftable stack itself which we'll write the
> > new list of tables to. But if we terminate abnormally e.g. via a call to
> > `die()`, then we do not remove the lockfile. Subsequent executions of
> > Git which try to modify references will thus fail with an out-of-date
> > error.
> >
> > Fix this bug by registering the lock as a `struct tempfile`, which
> > ensures automatic cleanup for us.
> 
> Makes sense.
> 
> > @@ -475,7 +471,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
> >  		goto done;
> >  	}
> >  	if (st->config.default_permissions) {
> > -		if (chmod(add->lock_file_name.buf, st->config.default_permissions) < 0) {
> > +		if (chmod(lock_file_name.buf, st->config.default_permissions) < 0) {
> 
> Hmm. Would we want to use add->lock_file->filename.buf here instead? I
> don't think that it matters (other than that the lockfile's pathname is
> absolute). But it arguably makes it clearer to readers that we're
> touching calling chmod() on the lockfile here, and hardens us against
> the contents of the temporary strbuf changing.

It doesn't make much of a difference, but I can see how this might make
things a bit clearer to the reader. Will change.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2023-12-11  9:08 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-21  7:04 [PATCH 0/8] reftable: small set of fixes Patrick Steinhardt
2023-11-21  7:04 ` [PATCH 1/8] reftable: wrap EXPECT macros in do/while Patrick Steinhardt
2023-11-21  7:04 ` [PATCH 2/8] reftable: handle interrupted reads Patrick Steinhardt
2023-11-21  7:04 ` [PATCH 3/8] reftable: handle interrupted writes Patrick Steinhardt
2023-11-21  7:04 ` [PATCH 4/8] reftable/stack: verify that `reftable_stack_add()` uses auto-compaction Patrick Steinhardt
2023-11-21  7:04 ` [PATCH 5/8] reftable/stack: perform auto-compaction with transactional interface Patrick Steinhardt
2023-12-21 10:29   ` Han-Wen Nienhuys
2023-12-21 10:45     ` Patrick Steinhardt
2023-11-21  7:04 ` [PATCH 6/8] reftable/stack: reuse buffers when reloading stack Patrick Steinhardt
2023-11-21  7:04 ` [PATCH 7/8] reftable/merged: reuse buffer to compute record keys Patrick Steinhardt
2023-12-21 10:48   ` Han-Wen Nienhuys
2023-11-21  7:04 ` [PATCH 8/8] reftable/stack: fix stale lock when dying Patrick Steinhardt
2023-12-08 14:52 ` [PATCH v2 00/11] reftable: small set of fixes Patrick Steinhardt
2023-12-08 14:52   ` [PATCH v2 01/11] reftable: wrap EXPECT macros in do/while Patrick Steinhardt
2023-12-08 14:53   ` [PATCH v2 02/11] reftable: handle interrupted reads Patrick Steinhardt
2023-12-08 21:30     ` Taylor Blau
2023-12-11  9:08       ` Patrick Steinhardt
2023-12-08 14:53   ` [PATCH v2 03/11] reftable: handle interrupted writes Patrick Steinhardt
2023-12-08 14:53   ` [PATCH v2 04/11] reftable/stack: verify that `reftable_stack_add()` uses auto-compaction Patrick Steinhardt
2023-12-08 21:35     ` Taylor Blau
2023-12-08 23:46       ` Eric Sunshine
2023-12-11  9:08         ` Patrick Steinhardt
2023-12-11  9:36           ` Eric Sunshine
2023-12-08 14:53   ` [PATCH v2 05/11] reftable/stack: perform auto-compaction with transactional interface Patrick Steinhardt
2023-12-08 22:14     ` Taylor Blau
2023-12-11  9:08       ` Patrick Steinhardt
2023-12-08 14:53   ` [PATCH v2 06/11] reftable/stack: reuse buffers when reloading stack Patrick Steinhardt
2023-12-08 22:17     ` Taylor Blau
2023-12-11  9:08       ` Patrick Steinhardt
2023-12-21 10:58         ` Han-Wen Nienhuys
2023-12-08 14:53   ` [PATCH v2 07/11] reftable/stack: fix stale lock when dying Patrick Steinhardt
2023-12-08 22:24     ` Taylor Blau
2023-12-11  9:08       ` Patrick Steinhardt [this message]
2023-12-08 14:53   ` [PATCH v2 08/11] reftable/stack: fix use of unseeded randomness Patrick Steinhardt
2023-12-21 10:49     ` Han-Wen Nienhuys
2023-12-08 14:53   ` [PATCH v2 09/11] reftable/merged: reuse buffer to compute record keys Patrick Steinhardt
2023-12-08 14:53   ` [PATCH v2 10/11] reftable/block: introduce macro to initialize `struct block_iter` Patrick Steinhardt
2023-12-08 14:53   ` [PATCH v2 11/11] reftable/block: reuse buffer to compute record keys Patrick Steinhardt
2023-12-21 10:43     ` Han-Wen Nienhuys
2023-12-28  5:53       ` Patrick Steinhardt
2023-12-08 22:26   ` [PATCH v2 00/11] reftable: small set of fixes Taylor Blau
2023-12-11  9:07 ` [PATCH v3 " Patrick Steinhardt
2023-12-11  9:07   ` [PATCH v3 01/11] reftable: wrap EXPECT macros in do/while Patrick Steinhardt
2023-12-11  9:07   ` [PATCH v3 02/11] reftable: handle interrupted reads Patrick Steinhardt
2023-12-11  9:07   ` [PATCH v3 03/11] reftable: handle interrupted writes Patrick Steinhardt
2023-12-11  9:07   ` [PATCH v3 04/11] reftable/stack: verify that `reftable_stack_add()` uses auto-compaction Patrick Steinhardt
2023-12-11 20:15     ` Taylor Blau
2023-12-12  3:44       ` Patrick Steinhardt
2023-12-11  9:07   ` [PATCH v3 05/11] reftable/stack: perform auto-compaction with transactional interface Patrick Steinhardt
2023-12-11  9:07   ` [PATCH v3 06/11] reftable/stack: reuse buffers when reloading stack Patrick Steinhardt
2023-12-11  9:07   ` [PATCH v3 07/11] reftable/stack: fix stale lock when dying Patrick Steinhardt
2023-12-11  9:07   ` [PATCH v3 08/11] reftable/stack: fix use of unseeded randomness Patrick Steinhardt
2023-12-11  9:08   ` [PATCH v3 09/11] reftable/merged: reuse buffer to compute record keys Patrick Steinhardt
2023-12-11  9:08   ` [PATCH v3 10/11] reftable/block: introduce macro to initialize `struct block_iter` Patrick Steinhardt
2023-12-11  9:08   ` [PATCH v3 11/11] reftable/block: reuse buffer to compute record keys Patrick Steinhardt
2023-12-11 20:16   ` [PATCH v3 00/11] reftable: small set of fixes Taylor Blau
2023-12-12  3:45     ` Patrick Steinhardt
2023-12-21 11:08 ` [PATCH 0/8] " Han-Wen Nienhuys

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=ZXbRoDh7j18olBDx@tanuki \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=hanwenn@gmail.com \
    --cc=jrnieder@gmail.com \
    --cc=me@ttaylorr.com \
    /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).