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
This commit is contained in:
Tom Tromey 2024-01-28 09:14:04 -07:00
parent ba9583c7d5
commit ed29a346be
3 changed files with 26 additions and 20 deletions

View File

@ -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

View File

@ -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;

View File

@ -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;