Bug 29854 - ^C should interrupt synchronous wait on cooked index completion
Summary: ^C should interrupt synchronous wait on cooked index completion
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: gdb (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: 14.1
Assignee: Tom Tromey
URL:
Keywords:
Depends on:
Blocks: 29366
  Show dependency treegraph
 
Reported: 2022-12-05 19:43 UTC by Simon Marchi
Modified: 2023-02-09 14:35 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
patch (827 bytes, patch)
2022-12-06 15:05 UTC, Tom Tromey
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Marchi 2022-12-05 19:43:41 UTC
Using commit 09a5d200e6166522e0d0a9276bd6b2227ac5ace1, I do:

$ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.gdb/selftest/xgdb
Reading symbols from testsuite/outputs/gdb.gdb/selftest/xgdb...
(gdb) b captured_main
^C

Creating the breakpoint takes some time, doing ^C does nothing.  Attaching to GDB, I understood that it's because the main thread is waiting on the `table->wait ()` call, waiting for worker threads to finish building the cooked indices.  Not sure how we can do it, but it would be nice for ^C to interrupt that table->wait call and generate a gdb_exception_quit.
Comment 1 Tom Tromey 2022-12-05 23:34:45 UTC
This seems fine as long as it is just the wait that is
interrupted and not the scan.
Comment 2 Tom Tromey 2022-12-06 00:00:09 UTC
There doesn't seem to be a good way to interrupt future::wait,
but I suppose the code could use the wait_for variant and check
the quit flag in a loop.

I'm also curious why this is taking long enough to notice.
Comment 3 Simon Marchi 2022-12-06 02:38:49 UTC
(In reply to Tom Tromey from comment #1)
> This seems fine as long as it is just the wait that is
> interrupted and not the scan.

Yes, that's what I was thinking, just interrupt the main thread's wait.

> There doesn't seem to be a good way to interrupt future::wait,
> but I suppose the code could use the wait_for variant and check
> the quit flag in a loop.

Meh :(.

> I'm also curious why this is taking long enough to notice.

I'm running my unoptimized build of GDB on itself (test gdb.gdb/selftest.exp).  So arguably not a very important use case.
Comment 4 Tom Tromey 2022-12-06 15:05:16 UTC
Created attachment 14484 [details]
patch

I couldn't really test this, but maybe you can?
Comment 5 Simon Marchi 2022-12-06 15:34:22 UTC
(In reply to Tom Tromey from comment #4)
> Created attachment 14484 [details]
> patch
> 
> I couldn't really test this, but maybe you can?

It works.

For fun, I added a "maintenance wait-finalize" command, that does:

static void
maintenance_wait_finalize (const char *args, int from_tty)
{
  inferior *inf = current_inferior ();

  for (objfile *o : inf->pspace->objfiles ())
    {
      dwarf_scanner_base *s
        = get_dwarf2_per_objfile (o)->per_bfd->index_table.get ();
      cooked_index_vector *v = dynamic_cast<cooked_index_vector *> (s);
      if (!v)
        continue;

      printf("Waiting for %s\n", objfile_name (o));
      v->wait ();
    }
}

This way, I can measure how much time GDB takes to make the index with:

$ /usr/bin/time ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.gdb/selftest/xgdb -ex "maint wait-finalize" -batch

With my unoptimized build, it takes about 30 seconds.  With my optimized build (on the same binary), it takes 2 seconds.  I noticed this, because I would start gdb.gdb/selftest.exp, then try to interrupt with ^C during the "break captured_main" command, and runtest would hang for a while, waiting for the GDB process to exit.

So, it's not really important to fix this, but it fixes an annoyance.
Comment 6 Tom Tromey 2022-12-06 19:09:34 UTC
> With my unoptimized build, it takes about 30 seconds.  With my optimized build
> (on the same binary), it takes 2 seconds.

Wow.  I knew optimization mattered for DWARF reading (maybe the only
spot it does for gdb), but I didn't realize the effect was so big :)
Comment 8 Sourceware Commits 2023-02-09 14:35:15 UTC
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=307733cc0fa864fcb92f4c308a0a27d17552df37

commit 307733cc0fa864fcb92f4c308a0a27d17552df37
Author: Tom Tromey <tromey@adacore.com>
Date:   Tue Dec 6 08:05:28 2022 -0700

    Let user C-c when waiting for DWARF index finalization
    
    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
Comment 9 Tom Tromey 2023-02-09 14:35:50 UTC
Fixed.