diff mbox series

[v2] Let user C-c when waiting for DWARF index finalization

Message ID 20230130200220.2879520-1-tromey@adacore.com
State New
Headers
Series [v2] Let user C-c when waiting for DWARF index finalization |

Commit Message

Tom Tromey Jan. 30, 2023, 8:02 p.m. UTC 
  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.

For v2, I realized that the thread compatibility code in thread-pool.h
also needed an update.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29854
---
 gdb/dwarf2/cooked-index.c | 16 ++++++++++++++++
 gdb/dwarf2/cooked-index.h | 11 +++++------
 gdbsupport/thread-pool.h  | 27 +++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 6 deletions(-)

Comments

Tom Tromey Feb. 9, 2023, 2:35 p.m. UTC | #1 
>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> In PR gdb/29854, Simon pointed out that it would be good to be able to
Tom> use C-c when the DWARF cooked index is waiting for finalization.  The
Tom> idea here is to be able to interrupt a command like "break" -- not to
Tom> stop the finalization process itself, which runs in a worker thread.

Tom> This patch implements this idea, by changing the index wait functions
Tom> to, by default, allow a quit.  Polling is done, because there doesn't
Tom> seem to be a better way to interrupt a wait on a std::future.

Tom> For v2, I realized that the thread compatibility code in thread-pool.h
Tom> also needed an update.

I've rebased this and made a small change for the cooked_index_shard
rename.  I also ensured it builds without threads.  I'm going to check
it in on trunk now.

Tom
diff mbox series

Patch

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 09d76523419..08a85104758 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -27,6 +27,7 @@ 
 #include <algorithm>
 #include "safe-ctype.h"
 #include "gdbsupport/selftest.h"
+#include <chrono>
 
 /* See cooked-index.h.  */
 
@@ -387,6 +388,21 @@  cooked_index::find (const std::string &name, bool completing) const
   return range (lower, upper);
 }
 
+/* See cooked-index.h.  */
+
+void
+cooked_index::wait (bool allow_quit) const
+{
+  if (allow_quit)
+    {
+      std::chrono::milliseconds duration { 15 };
+      while (m_future.wait_for (duration) == gdb::future_status::timeout)
+	QUIT;
+    }
+  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 891659f56e0..2461622ae17 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -255,10 +255,7 @@  class cooked_index
   void finalize ();
 
   /* Wait for this index's finalization to be complete.  */
-  void wait () const
-  {
-    m_future.wait ();
-  }
+  void wait (bool allow_quit = true) const;
 
   friend class cooked_index_vector;
 
@@ -369,8 +366,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.  */
diff --git a/gdbsupport/thread-pool.h b/gdbsupport/thread-pool.h
index 013c6abf8ea..cb8696e1fa4 100644
--- a/gdbsupport/thread-pool.h
+++ b/gdbsupport/thread-pool.h
@@ -23,6 +23,7 @@ 
 #include <queue>
 #include <vector>
 #include <functional>
+#include <chrono>
 #if CXX_STD_THREAD
 #include <thread>
 #include <mutex>
@@ -40,8 +41,19 @@  namespace gdb
 template<typename T>
 using future = std::future<T>;
 
+/* ... and the standard future_status.  */
+using future_status = std::future_status;
+
 #else /* CXX_STD_THREAD */
 
+/* A compatibility enum for std::future_status.  This is just the
+   subset needed by gdb.  */
+enum class future_status
+{
+  ready,
+  timeout,
+};
+
 /* A compatibility wrapper for std::future.  Once <thread> and
    <future> are available in all GCC builds -- should that ever happen
    -- this can be removed.  GCC does not implement threading for
@@ -71,6 +83,13 @@  class future
 
   void wait () const { }
 
+  template<class Rep, class Period>
+  future_status wait_for (const std::chrono::duration<Rep,Period> &duration)
+    const
+  {
+    return future_status::ready;
+  }
+
   T get () { return std::move (m_value); }
 
 private:
@@ -85,6 +104,14 @@  class future<void>
 {
 public:
   void wait () const { }
+
+  template<class Rep, class Period>
+  future_status wait_for (const std::chrono::duration<Rep,Period> &duration)
+    const
+  {
+    return future_status::ready;
+  }
+
   void get () { }
 };