public inbox for gdb-cvs@sourceware.org help / color / mirror / Atom feed
From: Tom Tromey <tromey@sourceware.org> To: gdb-cvs@sourceware.org Subject: [binutils-gdb] Avoid race when writing to index cache Date: Sat, 9 Mar 2024 00:27:39 +0000 (GMT) [thread overview] Message-ID: <20240309002739.CF04C385E036@sourceware.org> (raw) https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ed29a346be439466ff2a5ce33e715e02c49fbdac commit ed29a346be439466ff2a5ce33e715e02c49fbdac Author: Tom Tromey <tom@tromey.com> Date: Sun Jan 28 09:14:04 2024 -0700 Avoid race when writing to index cache The background DWARF reader changes introduced a race when writing to the index cache. The problem here is that constructing the index_cache_store_context object should only happen on the main thread, to ensure that the various value captures do not race. This patch adds an assert to the construct to that effect, and then arranges for this object to be constructed by the cooked_index_worker constructor -- which is only invoked on the main thread. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31262 Diff: --- gdb/dwarf2/cooked-index.c | 27 ++++++++++++--------------- gdb/dwarf2/cooked-index.h | 15 ++++++++++----- gdb/dwarf2/index-cache.c | 4 ++++ 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 5cbee83228b..2a1ca6fd225 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -578,6 +578,15 @@ cooked_index_worker::set (cooked_state desired_state) #endif /* CXX_STD_THREAD */ } +/* See cooked-index.h. */ + +void +cooked_index_worker::write_to_cache (const cooked_index *idx) const +{ + if (idx != nullptr) + m_cache_store.store (); +} + cooked_index::cooked_index (dwarf2_per_objfile *per_objfile, std::unique_ptr<cooked_index_worker> &&worker) : m_state (std::move (worker)), @@ -621,17 +630,16 @@ cooked_index::set_contents (vec_type &&vec) m_state->set (cooked_state::MAIN_AVAILABLE); - index_cache_store_context ctx (global_index_cache, m_per_bfd); - /* This is run after finalization is done -- but not before. If this task were submitted earlier, it would have to wait for finalization. However, that would take a slot in the global thread pool, and if enough such tasks were submitted at once, it would cause a livelock. */ - gdb::task_group finalizers ([this, ctx = std::move (ctx)] () + gdb::task_group finalizers ([this] () { m_state->set (cooked_state::FINALIZED); - maybe_write_index (ctx); + m_state->write_to_cache (index_for_writing ()); + m_state->set (cooked_state::CACHE_DONE); }); for (auto &idx : m_vector) @@ -836,17 +844,6 @@ cooked_index::dump (gdbarch *arch) } } -void -cooked_index::maybe_write_index (const index_cache_store_context &ctx) -{ - if (index_for_writing () != nullptr) - { - /* (maybe) store an index in the cache. */ - ctx.store (); - } - m_state->set (cooked_state::CACHE_DONE); -} - /* Wait for all the index cache entries to be written before gdb exits. */ static void diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index 17659df0531..91419b79edc 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -467,7 +467,8 @@ class cooked_index_worker public: explicit cooked_index_worker (dwarf2_per_objfile *per_objfile) - : m_per_objfile (per_objfile) + : m_per_objfile (per_objfile), + m_cache_store (global_index_cache, per_objfile->per_bfd) { } virtual ~cooked_index_worker () { } @@ -486,10 +487,15 @@ public: protected: - /* Let cooked_index call the 'set' method. */ + /* Let cooked_index call the 'set' and 'write_to_cache' methods. */ friend class cooked_index; + + /* Set the current state. */ void set (cooked_state desired_state); + /* Write to the index cache. */ + void write_to_cache (const cooked_index *idx) const; + /* Helper function that does the work of reading. This must be able to be run in a worker thread without problems. */ virtual void do_reading () = 0; @@ -534,6 +540,8 @@ protected: scanning is stopped and this exception will later be reported by the 'wait' method. */ std::optional<gdb_exception> m_failed; + /* An object used to write to the index cache. */ + index_cache_store_context m_cache_store; }; /* The main index of DIEs. @@ -671,9 +679,6 @@ public: private: - /* Maybe write the index to the index cache. */ - void maybe_write_index (const index_cache_store_context &); - /* The vector of cooked_index objects. This is stored because the entries are stored on the obstacks in those objects. */ vec_type m_vector; diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c index 780d2c4f200..d9047ef46bb 100644 --- a/gdb/dwarf2/index-cache.c +++ b/gdb/dwarf2/index-cache.c @@ -33,6 +33,7 @@ #include "gdbsupport/selftest.h" #include <string> #include <stdlib.h> +#include "run-on-main-thread.h" /* When set to true, show debug messages about the index cache. */ static bool debug_index_cache = false; @@ -94,6 +95,9 @@ index_cache_store_context::index_cache_store_context (const index_cache &ic, m_dir (ic.m_dir), m_per_bfd (per_bfd) { + /* Capturing globals may only be done on the main thread. */ + gdb_assert (is_main_thread ()); + if (!m_enabled) return;
reply other threads:[~2024-03-09 0:27 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20240309002739.CF04C385E036@sourceware.org \ --to=tromey@sourceware.org \ --cc=gdb-cvs@sourceware.org \ /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: linkBe 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 read-only IMAP folder(s) and NNTP newsgroup(s).