git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>
Subject: [PATCH 00/10] reftable: fix reload with active iterators
Date: Mon, 19 Aug 2024 17:39:38 +0200	[thread overview]
Message-ID: <cover.1724080006.git.ps@pks.im> (raw)

Hi,

as reported by Peff in [1], the reftable backend fails in t5616 in a
racy manner. The cause of this is that git-maintenance(1) processes leak
into subsequent tests and perform concurrent compaction of the stack.
The reftable backend is expected to handle this gracefully, but doesn't.

The issue can surface whenever reloading the reftable stack while an
iterator is active. In case some of the tables got compacted, we will
end up closing them and thus the iterator now tries to use those closed
tables.

This patch series fixes that issue by starting to refcount the readers.
Each iterator will bump the refcount, thus avoiding the problem. While
at it, I also found a second issue where we segfault when reloading a
table fails while reusing one of the table readers. In this scenario, we
would end up releasing the reader of the stack itself, even though it
would still be used by it.

This patch series addresses those issues by making the reftable reader
refcounted. The first 6 patches do some simplifications of code which is
close. The remaining 4 ones introduce refcounting and wire it up as
required.

With this, the following command now passes:

    make -C .. -j20 SANITIZE=address && GIT_TEST_DEFAULT_REF_FORMAT=reftable ./t5616-partial-clone.sh --run=1-16 --stress

The patch series builds on top of Junio's ps/reftable-drop-generic at
1a3c3870ee (reftable/generic: drop interface, 2024-08-14). It's only on
seen right now, but would otherise cause a bunch of conflicts.

Thanks!

[1]: <20240817121424.GA2439299@coredump.intra.peff.net>

Patrick Steinhardt (10):
  reftable/blocksource: drop malloc block source
  reftable/stack: inline `stack_compact_range_stats()`
  reftable/reader: rename `reftable_new_reader()`
  reftable/reader: inline `init_reader()`
  reftable/reader: inline `reader_close()`
  reftable/stack: fix broken refnames in `write_n_ref_tables()`
  reftable/reader: introduce refcounting
  reftable/reader: keep readers alive during iteration
  reftable/stack: reorder swapping in the reloaded stack contents
  reftable/stack: fix segfault when reload with reused readers fails

 reftable/block_test.c            |   3 +-
 reftable/blocksource.c           |  20 -----
 reftable/blocksource.h           |   2 -
 reftable/reader.c                | 149 ++++++++++++++++---------------
 reftable/reader.h                |   5 +-
 reftable/readwrite_test.c        |  85 +++++++++---------
 reftable/reftable-reader.h       |  19 ++--
 reftable/stack.c                 |  90 +++++++++++--------
 reftable/stack_test.c            | 115 +++++++++++++++++++++++-
 t/helper/test-reftable.c         |   4 +-
 t/unit-tests/t-reftable-merged.c |  10 +--
 11 files changed, 311 insertions(+), 191 deletions(-)


base-commit: 1a3c3870ee1a65b0579ccbac6b18e22b8c44c5b4
-- 
2.46.0.164.g477ce5ccd6.dirty


             reply	other threads:[~2024-08-19 15:39 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-19 15:39 Patrick Steinhardt [this message]
2024-08-19 15:39 ` [PATCH 01/10] reftable/blocksource: drop malloc block source Patrick Steinhardt
2024-08-19 15:39 ` [PATCH 02/10] reftable/stack: inline `stack_compact_range_stats()` Patrick Steinhardt
2024-08-22  8:23   ` karthik nayak
2024-08-19 15:39 ` [PATCH 03/10] reftable/reader: rename `reftable_new_reader()` Patrick Steinhardt
2024-08-22 18:51   ` Justin Tobler
2024-08-22 19:09     ` Junio C Hamano
2024-08-23  5:22       ` Patrick Steinhardt
2024-08-19 15:39 ` [PATCH 04/10] reftable/reader: inline `init_reader()` Patrick Steinhardt
2024-08-19 15:39 ` [PATCH 05/10] reftable/reader: inline `reader_close()` Patrick Steinhardt
2024-08-19 15:40 ` [PATCH 06/10] reftable/stack: fix broken refnames in `write_n_ref_tables()` Patrick Steinhardt
2024-08-19 15:40 ` [PATCH 07/10] reftable/reader: introduce refcounting Patrick Steinhardt
2024-08-22  9:47   ` karthik nayak
2024-08-22 11:35     ` Patrick Steinhardt
2024-08-23 10:14       ` karthik nayak
2024-08-19 15:40 ` [PATCH 08/10] reftable/reader: keep readers alive during iteration Patrick Steinhardt
2024-08-23 10:21   ` karthik nayak
2024-08-23 13:42     ` Patrick Steinhardt
2024-08-19 15:40 ` [PATCH 09/10] reftable/stack: reorder swapping in the reloaded stack contents Patrick Steinhardt
2024-08-19 15:40 ` [PATCH 10/10] reftable/stack: fix segfault when reload with reused readers fails Patrick Steinhardt
2024-08-22  8:13 ` [PATCH 00/10] reftable: fix reload with active iterators karthik nayak
2024-08-22 12:41 ` Jeff King
2024-08-22 12:53   ` Patrick Steinhardt
2024-08-22 15:15     ` Junio C Hamano
2024-08-23  5:23       ` Patrick Steinhardt
2024-08-22 15:11   ` Junio C Hamano
2024-08-23 14:12 ` [PATCH v2 " Patrick Steinhardt
2024-08-23 14:12   ` [PATCH v2 01/10] reftable/blocksource: drop malloc block source Patrick Steinhardt
2024-08-23 14:12   ` [PATCH v2 02/10] reftable/stack: inline `stack_compact_range_stats()` Patrick Steinhardt
2024-08-23 14:12   ` [PATCH v2 03/10] reftable/reader: rename `reftable_new_reader()` Patrick Steinhardt
2024-08-23 14:12   ` [PATCH v2 04/10] reftable/reader: inline `init_reader()` Patrick Steinhardt
2024-08-23 14:12   ` [PATCH v2 05/10] reftable/reader: inline `reader_close()` Patrick Steinhardt
2024-08-23 14:12   ` [PATCH v2 06/10] reftable/stack: fix broken refnames in `write_n_ref_tables()` Patrick Steinhardt
2024-08-23 14:12   ` [PATCH v2 07/10] reftable/reader: introduce refcounting Patrick Steinhardt
2024-08-23 14:12   ` [PATCH v2 08/10] reftable/reader: keep readers alive during iteration Patrick Steinhardt
2024-08-23 14:12   ` [PATCH v2 09/10] reftable/stack: reorder swapping in the reloaded stack contents Patrick Steinhardt
2024-08-23 14:12   ` [PATCH v2 10/10] reftable/stack: fix segfault when reload with reused readers fails Patrick Steinhardt
2024-08-23 16:49   ` [PATCH v2 00/10] reftable: fix reload with active iterators Junio C Hamano

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=cover.1724080006.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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).