Re: [PATCH] Fix race in background index-cache writing

[Simon Marchi <simark@simark.ca>]

On 3/24/23 17:55, Tom Tromey wrote:
> Tom de Vries pointed out a bug in the index-cache background writer --
> sometimes it will fail.  He also noted that it fails when the number
> of worker threads is set to zero.  These turn out to be the same
> problem -- the cache can't be written to until the per-BFD's
> "index_table" member is set.
> 
> This patch avoids the race by rearranging the code slightly, to ensure
> the cache cannot possibly be written before the member is set.

I've been seeing a lot of random failures of the index-cache tests
lately on my CI, so I presume this will help.

> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index c910be875a3..dbf323d0da0 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -5147,9 +5147,13 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
>    indexes.push_back (index_storage.release ());
>    indexes.shrink_to_fit ();
>  
> -  cooked_index *vec = new cooked_index (std::move (indexes), per_bfd);
> +  cooked_index *vec = new cooked_index (std::move (indexes));
>    per_bfd->index_table.reset (vec);
>  
> +  /* Cannot start writing the index entry until after the
> +     'index_table' member has been set.  */
> +  vec->start_writing_index (per_bfd);

An alternative I see would be to make cooked_index::cooked_index set the
per_bfd field itself (before starting the background task).  But if you
don't find it a clean solution for whatever reason, I think what you
have is fine.

Simon