Re: [PATCH v6 2/2] enable target async by default; separate MI and target notions of async

[Doug Evans <dje at google dot com>]

On Thu, May 29, 2014 at 6:48 AM, Pedro Alves <palves@redhat.com> wrote:
> [...]
> Below's what I pushed.
>
> 8<------------
> Subject: [PATCH] enable target async by default; separate MI and target
>  notions of async
>
> This finally makes background execution commands possible by default.
>
> However, in order to do that, there's one last thing we need to do --
> we need to separate the MI and target notions of "async".  Unlike the
> CLI, where the user explicitly requests foreground vs background
> execution in the execution command itself (c vs c&), MI chose to treat
> "set target-async" specially -- setting it changes the default
> behavior of execution commands.
>
> So, we can't simply "set target-async" default to on, as that would
> affect MI frontends.  Instead we have to make the setting MI-specific,
> and teach MI about sync commands on top of an async target.
>
> Because the "target" word in "set target-async" ends up as a potential
> source of confusion, the patch adds a "set mi-async" option, and makes
> "set target-async" a deprecated alias.
>
> Rather than make the targets always async, this patch introduces a new
> "maint set target-async" option so that the GDB developer can control
> whether the target is async.  This makes it simpler to debug issues
> arising only in the synchronous mode; important because sync mode
> seems unlikely to go away.
>
> Unlike in previous revisions, "set target-async" does not affect this
> new maint parameter.  The rationale for this is that then one can
> easily run the test suite in the "maint set target-async off" mode and
> have tests that enable mi-async fail just like they fail on
> non-async-capable targets.  This emulation is exactly the point of the
> maint option.
>
> I had asked Tom in a previous iteration to split the actual change of
> the target async default to a separate patch, but it turns out that
> that is quite awkward in this version of the patch, because with MI
> async and target async decoupled (unlike in previous versions), if we
> don't flip the default at the same time, then just "set target-async
> on" alone never actually manages to do anything.  It's best to not
> have that transitory state in the tree.
>
> Given "set target-async on" now only has effect for MI, the patch goes
> through the testsuite removing it from non-MI tests.  MI tests are
> adjusted to use the new and less confusing "mi-async" spelling.
>
> 2014-05-29  Pedro Alves  <palves@redhat.com>
>             Tom Tromey  <tromey@redhat.com>
>
>         * NEWS: Mention "maint set target-async", "set mi-async", and that
>         background execution commands are now always available.
>         * target.h (target_async_permitted): Update comment.
>         * target.c (target_async_permitted, target_async_permitted_1):
>         Default to 1.
>         (set_target_async_command): Rename to ...
>         (maint_set_target_async_command): ... this.
>         (show_target_async_command): Rename to ...
>         (maint_show_target_async_command): ... this.
>         (_initialize_target): Adjust.
>         * infcmd.c (prepare_execution_command): Make extern.
>         * inferior.h (prepare_execution_command): Declare.
>         * infrun.c (set_observer_mode): Leave target async alone.
>         * mi/mi-interp.c (mi_interpreter_init): Install
>         mi_on_sync_execution_done as sync_execution_done observer.
>         (mi_on_sync_execution_done): New function.
>         (mi_execute_command_input_handler): Don't print the prompt if we
>         just started a synchronous command with an async target.
>         (mi_on_resume): Check sync_execution before printing prompt.
>         * mi/mi-main.h (mi_async_p): Declare.
>         * mi/mi-main.c: Include gdbcmd.h.
>         (mi_async_p): New function.
>         (mi_async, mi_async_1): New globals.
>         (set_mi_async_command, show_mi_async_command, mi_async): New
>         functions.
>         (exec_continue): Call prepare_execution_command.
>         (run_one_inferior, mi_cmd_exec_run, mi_cmd_list_target_features)
>         (mi_execute_async_cli_command): Use mi_async_p.
>         (_initialize_mi_main): Install "set mi-async".  Make
>         "target-async" a deprecated alias.
>
> 2014-05-29  Pedro Alves  <palves@redhat.com>
>             Tom Tromey  <tromey@redhat.com>
>
>         * gdb.texinfo (Non-Stop Mode): Remove "set target-async 1"
>         from example.
>         (Asynchronous and non-stop modes): Document '-gdb-set mi-async'.
>         Mention that target-async is now deprecated.
>         (Maintenance Commands): Document maint set/show target-async.
>
> 2014-05-29  Pedro Alves  <palves@redhat.com>
>             Tom Tromey  <tromey@redhat.com>
>
>         * gdb.base/async-shell.exp: Don't enable target-async.
>         * gdb.base/async.exp
>         * gdb.base/corefile.exp (corefile_test_attach): Remove 'async'
>         parameter.  Adjust.
>         (top level): Don't test with "target-async".
>         * gdb.base/dprintf-non-stop.exp: Don't enable target-async.
>         * gdb.base/gdb-sigterm.exp: Don't test with "target-async".
>         * gdb.base/inferior-died.exp: Don't enable target-async.
>         * gdb.base/interrupt-noterm.exp: Likewise.
>         * gdb.mi/mi-async.exp: Use "mi-async" instead of "target-async".
>         * gdb.mi/mi-nonstop-exit.exp: Likewise.
>         * gdb.mi/mi-nonstop.exp: Likewise.
>         * gdb.mi/mi-ns-stale-regcache.exp: Likewise.
>         * gdb.mi/mi-nsintrall.exp: Likewise.
>         * gdb.mi/mi-nsmoribund.exp: Likewise.
>         * gdb.mi/mi-nsthrexec.exp: Likewise.
>         * gdb.mi/mi-watch-nonstop.exp: Likewise.
>         * gdb.multi/watchpoint-multi.exp: Adjust comment.
>         * gdb.python/py-evsignal.exp: Don't enable target-async.
>         * gdb.python/py-evthreads.exp: Likewise.
>         * gdb.python/py-prompt.exp: Likewise.
>         * gdb.reverse/break-precsave.exp: Don't test with "target-async".
>         * gdb.server/solib-list.exp: Don't enable target-async.
>         * gdb.threads/thread-specific-bp.exp: Likewise.
>         * lib/mi-support.exp: Adjust to use mi-async.

Hi.

I happened to notice a couple of comments that still need updating.

linux-nat.c:

/* target_is_async_p implementation.  */

static int
linux_nat_is_async_p (struct target_ops *ops)
{
  /* NOTE: palves 2008-03-21: We're only async when the user requests
     it explicitly with the "set target-async" command.
     Someday, linux will always be async.  */
  return target_async_permitted;
}

/* target_can_async_p implementation.  */

static int
linux_nat_can_async_p (struct target_ops *ops)
{
  /* NOTE: palves 2008-03-21: We're only async when the user requests
     it explicitly with the "set target-async" command.
     Someday, linux will always be async.  */
  return target_async_permitted;
}