[v1.1,01/23] Preserve selected thread in all-stop w/ background execution

Message ID c06fdff7-2f4a-f310-d6e0-23d00aab8cb6@redhat.com
State Superseded
Headers show
Series
  • [v1.1,01/23] Preserve selected thread in all-stop w/ background execution
Related show

Commit Message

Pedro Alves Oct. 16, 2019, 11:54 p.m.
On 10/9/19 10:36 AM, Aktemur, Tankut Baris wrote:
> * On September 7, 2019 1:28 AM, Pedro Alves wrote:

>>

>> diff --git a/gdb/infrun.c b/gdb/infrun.c

>> index a9588f896a..9c888aa72f 100644

>> --- a/gdb/infrun.c

>> +++ b/gdb/infrun.c

>> @@ -3048,6 +3048,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)

>>

>>    finish_state.release ();

>>

>> +  /* If we've switched threads above, switch back to the previously

>> +     current thread.  We don't want the user to see a different

>> +     selected thread.  */

>> +  switch_to_thread (cur_thr);

>> +

>>    /* Tell the event loop to wait for it to stop.  If the target

>>       supports asynchronous execution, it'll do this from within

>>       target_resume.  */

>> @@ -3702,14 +3707,11 @@ fetch_inferior_event (void *client_data)

>>  	set_current_traceframe (-1);

>>        }

>>

>> -    gdb::optional<scoped_restore_current_thread> maybe_restore_thread;

>> -

>> -    if (non_stop)

>> -      /* In non-stop mode, the user/frontend should not notice a thread

>> -	 switch due to internal events.  Make sure we reverse to the

>> -	 user selected thread and frame after handling the event and

>> -	 running any breakpoint commands.  */

>> -      maybe_restore_thread.emplace ();

>> +    /* The user/frontend should not notice a thread switch due to

>> +       internal events.  Make sure we revert to the user selected

>> +       thread and frame after handling the event and running any

>> +       breakpoint commands.  */

>> +    scoped_restore_current_thread restore_thread;

>>

> 

> Because this increases the refcount of the current thread, in case the

> fetched inferior event denotes a thread exit, the thread will not be deleted

> right away.  A non-deleted but exited thread stays in the inferior's thread

> list.  This, in turn, causes the "init_thread_list" call in inferior.c to

> be skipped.  As a side effect, a regression is observed in

> 

>   gdb.arch/i386-mpx-simple_segv.exp


Thanks for spotting this.  I don't have MPX on my machine, so I'm not
exactly sure the sequence of events that lead to a failure in that test,
but I found a way to reproduce a related problem without MPX, and wrote
a test based on it.

> 

> IMHO, the 'any_thread_p' predicate should be updated.  This predicate is used

> in two places (one in 'inferior.c' and the other in 'mi/mi-main.c').  Both

> uses, I believe, are in fact interested in whether there are any non-exited

> threads.  I'd suggest updating 'any_thread_p' to 'any_non_exited_thread_p'.

> 


I'm really not sure about that.  Exited threads have a thread number still,
as long as they're on the list.  Calling init_thread_list resets the
global thread counter, meaning that you could end up with multiple threads
with the same number, until the exited threads are purged.

I think instead a delete_exited_threads call in inferior_appeared
is better.

>>      overlay_cache_invalid = 1;

>>      /* Flush target cache before starting to handle each event.  Target

>> @@ -3786,6 +3788,19 @@ fetch_inferior_event (void *client_data)

>>  		inferior_event_handler (INF_EXEC_COMPLETE, NULL);

>>  		cmd_done = 1;

>>  	      }

>> +

>> +	    /* If we got a TARGET_WAITKIND_NO_RESUMED event, then the

>> +	       previously selected thread is gone.  We have two

>> +	       choices - switch to no thread selected, or restore the

>> +	       previously selected thread (now exited).  We chose the

>> +	       later, just because that's what GDB used to do.  After

>> +	       this, "info threads" says "The current thread <Thread

>> +	       ID 2> has terminated." instead of "No thread

>> +	       selected.".  */

>> +	    if (!non_stop

>> +		&& cmd_done

>> +		&& ecs->ws.kind != TARGET_WAITKIND_NO_RESUMED)

>> +	      restore_thread.dont_restore ();

>>  	  }

>>        }

>>

> 

> The comment and the code seem to contradict each other.  The comment says

> "if we got a TARGET_WAITKIND_NO_RESUMED" whereas the condition is

>   

>   ecs->ws.kind != TARGET_WAITKIND_NO_RESUMED


I don't think they're contradicting, actually.

When the 'if' condition is true, we won't restore the selected thread.
So we if we got a TARGET_WAITKIND_NO_RESUMED, we restore the previous
selected thread.  For all other event kinds, we won't restore.

> 

> Should TARGET_WAITKIND_THREAD_EXITED, TARGET_WAITKIND_EXITED, and

> TARGET_WAITKIND_SIGNALLED be also included in the condition?  They also mean

> that the thread is gone, right?


Not necessarily.  With checkpoint, gdb automatically switches to
another checkpoint on TARGET_WAITKIND_EXITED/TARGET_WAITKIND_SIGNALLED:

(gdb) checkpoint 
checkpoint 1: fork returned pid 10741.
(gdb) c
Continuing.
[Inferior 1 (process 10737) exited normally]
[Switching to process 10741]
(gdb) info threads 
  Id   Target Id            Frame 
* 1    process 10741 "main" main () at main.cc:21
(gdb) 

Here's an updated patch.  I believe it should fix the MPX issue too.

From 3de0c8d9df4f0979062718c745226e493206a305 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>

Date: Thu, 17 Oct 2019 00:37:02 +0100
Subject: [PATCH] Preserve selected thread in all-stop w/ background execution

In non-stop mode, if you resume the program in the background (with
"continue&", for example), then gdb makes sure to not switch the
current thread behind your back.  That means that you can be sure that
the commands you type apply to the thread you selected, even if some
other thread that was running in the background hits some event just
while you're typing.

In all-stop mode, however, if you resume the program in the
background, gdb let's the current thread switch behind your back.

This is bogus, of course.  All-stop and non-stop background
resumptions should behave the same.

This patch fixes that, and adds a testcase that exposes the bad
behavior in current master.

The fork-running-state.exp changes are necessary because that
preexisting testcase was expecting the old behavior:

Before:

  continue &
  Continuing.
  (gdb)
  [Attaching after process 8199 fork to child process 8203]
  [New inferior 2 (process 8203)]
  info threads
    Id   Target Id                      Frame
    1.1  process 8199 "fork-running-st" (running)
  * 2.1  process 8203 "fork-running-st" (running)
  (gdb)

After:

  continue &
  Continuing.
  (gdb)
  [Attaching after process 24660 fork to child process 24664]
  [New inferior 2 (process 24664)]
  info threads
    Id   Target Id                       Frame
  * 1.1  process 24660 "fork-running-st" (running)
    2.1  process 24664 "fork-running-st" (running)
  (gdb)

Here we see that before this patch GDB switches current inferior to
the new inferior behind the user's back, as a side effect of handling
the fork.

The delete_exited_threads call in inferior_appeared is there to fix an
issue that Baris found in a previous version of this patch.  The
fetch_inferior_event change increases the refcount of the current
thread, and in case the fetched inferior event denotes a thread exit,
the thread will not be deleted right away.  A non-deleted but exited
thread stays in the inferior's thread list.  This, in turn, causes the
"init_thread_list" call in inferior.c to be skipped.  A consequence is
that the global thread ID counter is not restarted if the current
thread exits, and then the inferior is restarted:

 (gdb) start
 Temporary breakpoint 1 at 0x4004d6: file main.c, line 21.
 Starting program: /tmp/main

 Temporary breakpoint 1, main () at main.c:21
 21        foo ();
 (gdb) info threads -gid
   Id   GId  Target Id            Frame
 * 1    1    process 16106 "main" main () at main.c:21
 (gdb) c
 Continuing.
 [Inferior 1 (process 16106) exited normally]
 (gdb) start
 Temporary breakpoint 2 at 0x4004d6: file main.c, line 21.
 Starting program: /tmp/main

 Temporary breakpoint 2, main () at main.c:21
 21        foo ();
 (gdb) info threads -gid
   Id   GId  Target Id            Frame
 * 1    2    process 16138 "main" main () at main.c:21
       ^^^

Notice that GId == 2 above.  It should have been "1" instead.

The new tids-git-reset.exp testcase exercises the problem above.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdbthread.h (scoped_restore_current_thread)
	<dont_restore, restore, m_dont_restore>: Declare.
	* thread.c (thread_alive): Add assertion.  Return bool.
	(switch_to_thread_if_alive): New.
	(prune_threads): Switch inferior/thread.
	(print_thread_info_1): Switch thread before calling target methods.
	(scoped_restore_current_thread::restore): New, factored out from
	...
	(scoped_restore_current_thread::~scoped_restore_current_thread):
	... this.
	(scoped_restore_current_thread::scoped_restore_current_thread):
	Add assertion.
	(thread_apply_all_command, thread_select): Use
	switch_to_thread_if_alive.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/fork-running-state.exp (do_test): Adjust expected
	output.
	* gdb.threads/async.c: New.
	* gdb.threads/async.exp: New.
	* gdb.multi/tids-gid-reset.c: New.
	* gdb.multi/tids-gid-reset.exp: New.
---
 gdb/gdbthread.h                               |  6 ++
 gdb/inferior.c                                |  1 +
 gdb/infrun.c                                  | 31 ++++++---
 gdb/testsuite/gdb.base/fork-running-state.exp | 17 +----
 gdb/testsuite/gdb.multi/tids-gid-reset.c      | 22 ++++++
 gdb/testsuite/gdb.multi/tids-gid-reset.exp    | 96 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.threads/async.c             | 70 +++++++++++++++++++
 gdb/testsuite/gdb.threads/async.exp           | 98 +++++++++++++++++++++++++++
 gdb/thread.c                                  | 19 +++++-
 9 files changed, 337 insertions(+), 23 deletions(-)
 create mode 100644 gdb/testsuite/gdb.multi/tids-gid-reset.c
 create mode 100644 gdb/testsuite/gdb.multi/tids-gid-reset.exp
 create mode 100644 gdb/testsuite/gdb.threads/async.c
 create mode 100644 gdb/testsuite/gdb.threads/async.exp

-- 
2.14.5

Comments

Tankut Baris Aktemur Oct. 17, 2019, 10:21 a.m. | #1
* On Thursday, October 17, 2019 1:55 AM, Pedro Alves wrote:
> 

> On 10/9/19 10:36 AM, Aktemur, Tankut Baris wrote:

> > * On September 7, 2019 1:28 AM, Pedro Alves wrote:

> >>

> >> diff --git a/gdb/infrun.c b/gdb/infrun.c

> >> index a9588f896a..9c888aa72f 100644

> >> --- a/gdb/infrun.c

> >> +++ b/gdb/infrun.c

> >> @@ -3048,6 +3048,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)

> >>

> >>    finish_state.release ();

> >>

> >> +  /* If we've switched threads above, switch back to the previously

> >> +     current thread.  We don't want the user to see a different

> >> +     selected thread.  */

> >> +  switch_to_thread (cur_thr);

> >> +

> >>    /* Tell the event loop to wait for it to stop.  If the target

> >>       supports asynchronous execution, it'll do this from within

> >>       target_resume.  */

> >> @@ -3702,14 +3707,11 @@ fetch_inferior_event (void *client_data)

> >>  	set_current_traceframe (-1);

> >>        }

> >>

> >> -    gdb::optional<scoped_restore_current_thread> maybe_restore_thread;

> >> -

> >> -    if (non_stop)

> >> -      /* In non-stop mode, the user/frontend should not notice a thread

> >> -	 switch due to internal events.  Make sure we reverse to the

> >> -	 user selected thread and frame after handling the event and

> >> -	 running any breakpoint commands.  */

> >> -      maybe_restore_thread.emplace ();

> >> +    /* The user/frontend should not notice a thread switch due to

> >> +       internal events.  Make sure we revert to the user selected

> >> +       thread and frame after handling the event and running any

> >> +       breakpoint commands.  */

> >> +    scoped_restore_current_thread restore_thread;

> >>

> >

> > Because this increases the refcount of the current thread, in case the

> > fetched inferior event denotes a thread exit, the thread will not be deleted

> > right away.  A non-deleted but exited thread stays in the inferior's thread

> > list.  This, in turn, causes the "init_thread_list" call in inferior.c to

> > be skipped.  As a side effect, a regression is observed in

> >

> >   gdb.arch/i386-mpx-simple_segv.exp

> 

> Thanks for spotting this.  I don't have MPX on my machine, so I'm not

> exactly sure the sequence of events that lead to a failure in that test,

> but I found a way to reproduce a related problem without MPX, and wrote

> a test based on it.

> 

> >

> > IMHO, the 'any_thread_p' predicate should be updated.  This predicate is used

> > in two places (one in 'inferior.c' and the other in 'mi/mi-main.c').  Both

> > uses, I believe, are in fact interested in whether there are any non-exited

> > threads.  I'd suggest updating 'any_thread_p' to 'any_non_exited_thread_p'.

> >

> 

> I'm really not sure about that.  Exited threads have a thread number still,

> as long as they're on the list.  Calling init_thread_list resets the

> global thread counter, meaning that you could end up with multiple threads

> with the same number, until the exited threads are purged.

> 

> I think instead a delete_exited_threads call in inferior_appeared

> is better.


Thanks, this makes sense.

> 

> >>      overlay_cache_invalid = 1;

> >>      /* Flush target cache before starting to handle each event.  Target

> >> @@ -3786,6 +3788,19 @@ fetch_inferior_event (void *client_data)

> >>  		inferior_event_handler (INF_EXEC_COMPLETE, NULL);

> >>  		cmd_done = 1;

> >>  	      }

> >> +

> >> +	    /* If we got a TARGET_WAITKIND_NO_RESUMED event, then the

> >> +	       previously selected thread is gone.  We have two

> >> +	       choices - switch to no thread selected, or restore the

> >> +	       previously selected thread (now exited).  We chose the

> >> +	       later, just because that's what GDB used to do.  After

> >> +	       this, "info threads" says "The current thread <Thread

> >> +	       ID 2> has terminated." instead of "No thread

> >> +	       selected.".  */

> >> +	    if (!non_stop

> >> +		&& cmd_done

> >> +		&& ecs->ws.kind != TARGET_WAITKIND_NO_RESUMED)

> >> +	      restore_thread.dont_restore ();

> >>  	  }

> >>        }

> >>

> >

> > The comment and the code seem to contradict each other.  The comment says

> > "if we got a TARGET_WAITKIND_NO_RESUMED" whereas the condition is

> >

> >   ecs->ws.kind != TARGET_WAITKIND_NO_RESUMED

> 

> I don't think they're contradicting, actually.

> 

> When the 'if' condition is true, we won't restore the selected thread.

> So we if we got a TARGET_WAITKIND_NO_RESUMED, we restore the previous

> selected thread.  For all other event kinds, we won't restore.

> 


Right, sorry, my bad.

> >

> > Should TARGET_WAITKIND_THREAD_EXITED, TARGET_WAITKIND_EXITED, and

> > TARGET_WAITKIND_SIGNALLED be also included in the condition?  They also mean

> > that the thread is gone, right?

> 

> Not necessarily.  With checkpoint, gdb automatically switches to

> another checkpoint on TARGET_WAITKIND_EXITED/TARGET_WAITKIND_SIGNALLED:

> 

> (gdb) checkpoint

> checkpoint 1: fork returned pid 10741.

> (gdb) c

> Continuing.

> [Inferior 1 (process 10737) exited normally]

> [Switching to process 10741]

> (gdb) info threads

>   Id   Target Id            Frame

> * 1    process 10741 "main" main () at main.cc:21

> (gdb)

> 

> Here's an updated patch.  I believe it should fix the MPX issue too.


Yes, the MPX test no longer regresses with this updated patch.

Regards,
-Baris

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Patch

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 370141e688..62b73fb83b 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -645,7 +645,13 @@  public:
 
   DISABLE_COPY_AND_ASSIGN (scoped_restore_current_thread);
 
+  /* Cancel restoring on scope exit.  */
+  void dont_restore () { m_dont_restore = true; }
+
 private:
+  void restore ();
+
+  bool m_dont_restore = false;
   /* Use the "class" keyword here, because of a clash with a "thread_info"
      function in the Darwin API.  */
   class thread_info *m_thread;
diff --git a/gdb/inferior.c b/gdb/inferior.c
index cf2175494d..fa1fd3fc10 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -255,6 +255,7 @@  inferior_appeared (struct inferior *inf, int pid)
 {
   /* If this is the first inferior with threads, reset the global
      thread id.  */
+  delete_exited_threads ();
   if (!any_thread_p ())
     init_thread_list ();
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 07aebfa678..6da95e0584 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3046,6 +3046,11 @@  proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 
   finish_state.release ();
 
+  /* If we've switched threads above, switch back to the previously
+     current thread.  We don't want the user to see a different
+     selected thread.  */
+  switch_to_thread (cur_thr);
+
   /* Tell the event loop to wait for it to stop.  If the target
      supports asynchronous execution, it'll do this from within
      target_resume.  */
@@ -3700,14 +3705,11 @@  fetch_inferior_event (void *client_data)
 	set_current_traceframe (-1);
       }
 
-    gdb::optional<scoped_restore_current_thread> maybe_restore_thread;
-
-    if (non_stop)
-      /* In non-stop mode, the user/frontend should not notice a thread
-	 switch due to internal events.  Make sure we reverse to the
-	 user selected thread and frame after handling the event and
-	 running any breakpoint commands.  */
-      maybe_restore_thread.emplace ();
+    /* The user/frontend should not notice a thread switch due to
+       internal events.  Make sure we revert to the user selected
+       thread and frame after handling the event and running any
+       breakpoint commands.  */
+    scoped_restore_current_thread restore_thread;
 
     overlay_cache_invalid = 1;
     /* Flush target cache before starting to handle each event.  Target
@@ -3784,6 +3786,19 @@  fetch_inferior_event (void *client_data)
 		inferior_event_handler (INF_EXEC_COMPLETE, NULL);
 		cmd_done = 1;
 	      }
+
+	    /* If we got a TARGET_WAITKIND_NO_RESUMED event, then the
+	       previously selected thread is gone.  We have two
+	       choices - switch to no thread selected, or restore the
+	       previously selected thread (now exited).  We chose the
+	       later, just because that's what GDB used to do.  After
+	       this, "info threads" says "The current thread <Thread
+	       ID 2> has terminated." instead of "No thread
+	       selected.".  */
+	    if (!non_stop
+		&& cmd_done
+		&& ecs->ws.kind != TARGET_WAITKIND_NO_RESUMED)
+	      restore_thread.dont_restore ();
 	  }
       }
 
diff --git a/gdb/testsuite/gdb.base/fork-running-state.exp b/gdb/testsuite/gdb.base/fork-running-state.exp
index 98733a6cc1..143b5ce714 100644
--- a/gdb/testsuite/gdb.base/fork-running-state.exp
+++ b/gdb/testsuite/gdb.base/fork-running-state.exp
@@ -98,30 +98,19 @@  proc do_test { detach_on_fork follow_fork non_stop schedule_multiple } {
 
     set not_nl "\[^\r\n\]*"
 
-    if {$detach_on_fork == "on" && $non_stop == "on" && $follow_fork == "child"} {
+    if {$detach_on_fork == "on" && $follow_fork == "child"} {
 	gdb_test "info threads" \
 	    "  2.1 ${not_nl}\\\(running\\\).*No selected thread.*"
-    } elseif {$detach_on_fork == "on" && $follow_fork == "child"} {
-	gdb_test "info threads" \
-	    "\\\* 2.1 ${not_nl}\\\(running\\\)"
     } elseif {$detach_on_fork == "on"} {
 	gdb_test "info threads" \
 	    "\\\* 1 ${not_nl}\\\(running\\\)"
-    } elseif {$non_stop == "on"
-	      || ($schedule_multiple == "on" && $follow_fork == "parent")} {
+    } elseif {$non_stop == "on" || $schedule_multiple == "on"} {
 	# Both parent and child should be marked running, and the
 	# parent should be selected.
 	gdb_test "info threads" \
 	    [multi_line \
 		 "\\\* 1.1 ${not_nl} \\\(running\\\)${not_nl}" \
 		 "  2.1 ${not_nl} \\\(running\\\)"]
-    } elseif {$schedule_multiple == "on" && $follow_fork == "child"} {
-	# Both parent and child should be marked running, and the
-	# child should be selected.
-	gdb_test "info threads" \
-	    [multi_line \
-		 "  1.1 ${not_nl} \\\(running\\\)${not_nl}" \
-		 "\\\* 2.1 ${not_nl} \\\(running\\\)"]
     } else {
 	set test "only $follow_fork marked running"
 	gdb_test_multiple "info threads" $test {
@@ -131,7 +120,7 @@  proc do_test { detach_on_fork follow_fork non_stop schedule_multiple } {
 	    -re "\\\* 1.1 ${not_nl}\\\(running\\\)\r\n  2.1 ${not_nl}\r\n$gdb_prompt $" {
 		gdb_assert [string eq $follow_fork "parent"] $test
 	    }
-	    -re "1.1 ${not_nl}\r\n\\\* 2.1 ${not_nl}\\\(running\\\)\r\n$gdb_prompt $" {
+	    -re "\\\* 1.1 ${not_nl}\r\n  2.1 ${not_nl}\\\(running\\\)\r\n$gdb_prompt $" {
 		gdb_assert [string eq $follow_fork "child"] $test
 	    }
 	}
diff --git a/gdb/testsuite/gdb.multi/tids-gid-reset.c b/gdb/testsuite/gdb.multi/tids-gid-reset.c
new file mode 100644
index 0000000000..4d3d67b8e4
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/tids-gid-reset.c
@@ -0,0 +1,22 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.multi/tids-gid-reset.exp b/gdb/testsuite/gdb.multi/tids-gid-reset.exp
new file mode 100644
index 0000000000..e15049bfad
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/tids-gid-reset.exp
@@ -0,0 +1,96 @@ 
+# Copyright 2015-2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that letting the inferior exit and restarting it again resets
+# the global TID counter, and thus the new thread 1.1 should end up
+# with global TID == 1.
+#
+# Also, check the same but with another inferior still running, in
+# which case the new thread 1.1 should end up with global TID == 3.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {pthreads debug}] } {
+    return -1
+}
+
+with_test_prefix "single-inferior" {
+    with_test_prefix "before restart" {
+	clean_restart ${testfile}
+
+	if { ![runto_main] } then {
+	    return -1
+	}
+
+	gdb_test "info threads -gid" "\\* 1 +1 +.*"
+    }
+
+    with_test_prefix "restart" {
+	gdb_continue_to_end
+	if { ![runto_main] } then {
+	    return -1
+	}
+    }
+
+    with_test_prefix "after restart" {
+	gdb_test "info threads -gid" "\\* 1 +1 +.*"
+    }
+}
+
+# For the following tests, multiple inferiors are needed, therefore
+# non-extended gdbserver is not supported.
+if [use_gdb_stub] {
+    untested "using gdb stub"
+    return
+}
+
+# Test with multiple inferiors.  This time, since we restart inferior
+# 1 while inferior 2 still has threads, then the new thread 1.1 should
+# end up with GID == 3, since we won't be able to reset the global
+# thread ID counter.
+with_test_prefix "multi-inferior" {
+    gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2"
+    gdb_test "inferior 2" "Switching to inferior 2 .*" "switch to inferior 2"
+    gdb_load ${binfile}
+
+    if ![runto_main] then {
+	fail "starting inferior 2"
+	return
+    }
+
+    gdb_test "inferior 1" "Switching to inferior 1 .*" \
+	"switch back to inferior 1"
+
+    with_test_prefix "before restart" {
+	gdb_test "info threads -gid" \
+	    [multi_line \
+		 "\\* 1\.1 +1 +.*" \
+		 "  2\.1 +2 +.*"]
+    }
+
+    with_test_prefix "restart" {
+	gdb_continue_to_end
+	if { ![runto_main] } then {
+	    return -1
+	}
+    }
+
+    with_test_prefix "after restart" {
+	gdb_test "info threads -gid" \
+	    [multi_line \
+		 "\\* 1\.1 +3 +.*" \
+		 "  2\.1 +2 +.*"]
+    }
+}
diff --git a/gdb/testsuite/gdb.threads/async.c b/gdb/testsuite/gdb.threads/async.c
new file mode 100644
index 0000000000..6bffca9c8d
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/async.c
@@ -0,0 +1,70 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+#define NUM 2
+
+static pthread_barrier_t threads_started_barrier;
+
+static void *
+thread_function (void *arg)
+{
+  pthread_barrier_wait (&threads_started_barrier);
+
+  while (1)
+    {
+      /* Sleep a bit to give the other threads a chance to run.  */
+      usleep (1); /* set breakpoint here */
+    }
+
+  pthread_exit (NULL);
+}
+
+static void
+all_started (void)
+{
+}
+
+int
+main ()
+{
+  pthread_t threads[NUM];
+  long i;
+
+  pthread_barrier_init (&threads_started_barrier, NULL, NUM + 1);
+
+  for (i = 1; i <= NUM; i++)
+    {
+      int res;
+
+      res = pthread_create (&threads[i - 1],
+			    NULL,
+			    thread_function, NULL);
+    }
+
+  pthread_barrier_wait (&threads_started_barrier);
+
+  all_started ();
+
+  sleep (180);
+
+  exit (EXIT_SUCCESS);
+}
diff --git a/gdb/testsuite/gdb.threads/async.exp b/gdb/testsuite/gdb.threads/async.exp
new file mode 100644
index 0000000000..7047bef2bd
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/async.exp
@@ -0,0 +1,98 @@ 
+# Copyright (C) 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile {debug pthreads}] == -1} {
+    return -1
+}
+
+# At this point GDB will be busy handling the breakpoint hits and
+# re-resuming the program.  Even if GDB internally switches thread
+# context, the user should not notice it.  The following part of the
+# testcase ensures that.
+
+# Switch to thread EXPECTED_THR, and then confirm that the thread
+# stays selected.
+
+proc test_current_thread {expected_thr} {
+    global decimal
+    global gdb_prompt
+    global binfile
+
+    clean_restart $binfile
+
+    if {![runto "all_started"]} {
+	fail "could not run to all_started"
+	return
+    }
+
+    # Set a breakpoint that continuously fires but doeesn't cause a stop.
+    gdb_breakpoint [concat [gdb_get_line_number "set breakpoint here"] " if 0"]
+
+    gdb_test "thread $expected_thr" "Switching to thread $expected_thr .*" \
+	"switch to thread $expected_thr"
+
+    # Continue the program in the background.
+    set test "continue&"
+    gdb_test_multiple "continue&" $test {
+	-re "Continuing\\.\r\n$gdb_prompt " {
+	    pass $test
+	}
+    }
+
+    set test "current thread is $expected_thr"
+    set fails 0
+    for {set i 0} {$i < 10} {incr i} {
+	after 200
+
+	set cur_thread 0
+	gdb_test_multiple "thread" $test {
+	    -re "Current thread is ($decimal) .*$gdb_prompt " {
+		set cur_thread $expect_out(1,string)
+	    }
+	}
+
+	if {$cur_thread != $expected_thr} {
+	    incr fails
+	}
+    }
+
+    gdb_assert {$fails == 0} $test
+
+    # Explicitly interrupt the target, because in all-stop/remote,
+    # that's all we can do when the target is running.  If we don't do
+    # this, we'd time out trying to kill the target, while bringing
+    # down gdb & gdbserver.
+    set test "interrupt"
+    gdb_test_multiple $test $test {
+	-re "^interrupt\r\n$gdb_prompt " {
+	    gdb_test_multiple "" $test {
+		-re "Thread .* received signal SIGINT, Interrupt\\." {
+		    pass $test
+		}
+	    }
+	}
+    }
+}
+
+# Try once with each thread as current, to avoid missing a bug just
+# because some part of GDB manages to switch to the right thread by
+# chance.
+for {set thr 1} {$thr <= 3} {incr thr} {
+    with_test_prefix "thread $thr" {
+	test_current_thread $thr
+    }
+}
diff --git a/gdb/thread.c b/gdb/thread.c
index 17bc642b84..2f9cd87ac8 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1375,7 +1375,8 @@  restore_selected_frame (struct frame_id a_frame_id, int frame_level)
     }
 }
 
-scoped_restore_current_thread::~scoped_restore_current_thread ()
+void
+scoped_restore_current_thread::restore ()
 {
   /* If an entry of thread_info was previously selected, it won't be
      deleted because we've increased its refcount.  The thread represented
@@ -1402,6 +1403,22 @@  scoped_restore_current_thread::~scoped_restore_current_thread ()
       && target_has_stack
       && target_has_memory)
     restore_selected_frame (m_selected_frame_id, m_selected_frame_level);
+}
+
+scoped_restore_current_thread::~scoped_restore_current_thread ()
+{
+  if (!m_dont_restore)
+    {
+      try
+	{
+	  restore ();
+	}
+      catch (const gdb_exception &ex)
+	{
+	  /* We're in a dtor, there's really nothing else we can do
+	     but swallow the exception.  */
+	}
+    }
 
   if (m_thread != NULL)
     m_thread->decref ();