Re: [PATCH] Let user C-c when waiting for DWARF index finalization

[Lancelot SIX <lsix@lancelotsix.com>]

On Tue, Dec 06, 2022 at 12:27:43PM -0700, Tom Tromey via Gdb-patches wrote:
> In PR gdb/29854, Simon pointed out that it would be good to be able to
> use C-c when the DWARF cooked index is waiting for finalization.  The
> idea here is to be able to interrupt a command like "break" -- not to
> stop the finalization process itself, which runs in a worker thread.
> 
> This patch implements this idea, by changing the index wait functions
> to, by default, allow a quit.  Polling is done, because there doesn't
> seem to be a better way to interrupt a wait on a std::future.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29854
> ---
>  gdb/dwarf2/cooked-index.c | 17 +++++++++++++++++
>  gdb/dwarf2/cooked-index.h | 11 +++++------
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
> index 0aa026c7779..a0b75a311b1 100644
> --- a/gdb/dwarf2/cooked-index.c
> +++ b/gdb/dwarf2/cooked-index.c
> @@ -25,6 +25,7 @@
>  #include "ada-lang.h"
>  #include "split-name.h"
>  #include <algorithm>
> +#include <chrono>
>  
>  /* Hash function for cooked_index_entry.  */
>  
> @@ -282,6 +283,22 @@ cooked_index::find (gdb::string_view name, bool completing)
>    return range (lower, upper);
>  }
>  
> +/* See cooked-index.h.  */
> +
> +void
> +cooked_index::wait (bool allow_quit)
> +{
> +  if (allow_quit)
> +    {
> +      using namespace std::chrono_literals;
> +      auto duration = 15ms;
> +      while (m_future.wait_for (duration) == std::future_status::timeout)
> +	QUIT;

Hi,

Thanks for doing this!

I think the "ms" suffix from chrono_literals have been introduced in
c++14 which we do not support. 

I guess that something like this should work for c++11:

    std::chrono::milliseconds duration { 15 };

I still have on my todo list to ensure that major distros do have a
decent and easily installable c++14 compiler so we can bump this
requirement in gdb, but I have not done this yet…

Except for this, and for what it is worth, this patch looks nice!

Best,
Lancelot.

> +    }
> +  else
> +    m_future.wait ();
> +}
> +
>  cooked_index_vector::cooked_index_vector (vec_type &&vec)
>    : m_vector (std::move (vec))
>  {
> diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
> index 2ea32781be5..5412149588a 100644
> --- a/gdb/dwarf2/cooked-index.h
> +++ b/gdb/dwarf2/cooked-index.h
> @@ -212,10 +212,7 @@ class cooked_index
>    void finalize ();
>  
>    /* Wait for this index's finalization to be complete.  */
> -  void wait ()
> -  {
> -    m_future.wait ();
> -  }
> +  void wait (bool allow_quit = true);
>  
>    friend class cooked_index_vector;
>  
> @@ -325,8 +322,10 @@ class cooked_index_vector : public dwarf_scanner_base
>         end up writing to freed memory.  Waiting for finalization to
>         complete avoids this problem; and the cost seems ignorable
>         because creating and immediately destroying the debug info is a
> -       relatively rare thing to do.  */
> -    wait ();
> +       relatively rare thing to do.  Do not allow quitting from this
> +       wait.  */
> +    for (auto &item : m_vector)
> +      item->wait (false);
>    }
>  
>    /* A range over a vector of subranges.  */
> -- 
> 2.34.3
>