public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@palves.net>
To: gdb-patches@sourceware.org
Subject: [PATCH 09/11] Ensure EXIT is last event, gdb/linux
Date: Thu,  3 Mar 2022 14:40:18 +0000	[thread overview]
Message-ID: <20220303144020.3601082-10-pedro@palves.net> (raw)
In-Reply-To: <20220303144020.3601082-1-pedro@palves.net>

From: Lancelot SIX <lancelot.six@amd.com>

When all threads of a multi-threaded process terminate, we can end up
with multiple LWPs having a pending exit event (lets say that the
events arrive faster than GDB processes them, which can be simulated
by introducing an artificial delay in GDB).  When we have multiple
pending events to report to the core, linux_nat_wait_1 uses
select_event_lwp to select randomly one of the pending events to
report.

If we have multiple pending exit events, and the randomization picks
the leader's exit to report, filter_exit_event sees that this is the
leader exiting, thus it is the exit of the whole process and thus
reports an EXITED event to the core, while leaving the other threads'
exit statuses pending.

This is problematic for infrun.c:stop_all_threads, which asks the
target to report all thread exit events to infrun.  For example, in
stop_all_threads, core GDB counts 2 threads that needs to be stopped.
It then asks the target to stop those 2 threads (with
target_stop(ptid)), and waits for 2 events to come back from the
target.  Unfortunately, when waiting for events, the linux-nat target,
due to the random event selecting mentioned above, reports the
whole-process EXIT event even though the other thread has exited and
its exit hasn't been reported yet.  As a consequence, stop_all_threads
receives one event, but waits indefinitely for the second one to come.
Effectively, GDB hangs forever.

To fix this, this commit makes sure that a leader's exit event is not
considered for selection by select_event_lwp as long as there is at
least one other thread with a pending event remaining in the process.
Considering that the leader thread's exit event can only be generated
by the kernel once it has reaped all the non-leader threads, we are
guaranteed that all other threads do have an exit event which is ready
to be processed.  Once all other exit events are processed,
select_event_lwp will consider the leader's exit status.

Tested on Linux-x86_64 with no regression observed.

Co-Authored-By: Pedro Alves <pedro@palves.net>
Change-Id: Id17ad5de76518925a968c0902860646820679dfa
---
 gdb/linux-nat.c | 46 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index d97a770bf83..5d8887b11f6 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2482,15 +2482,50 @@ status_callback (struct lwp_info *lp)
   return 1;
 }
 
-/* Count the LWP's that have had events.  */
+/* Return true if another thread of the same process as LP has a
+   pending status ready to be processed.  LP is assumed to be the
+   leader of its process.  */
+
+static bool
+non_leader_lwp_in_process_with_pending_event (lwp_info *lp)
+{
+  gdb_assert (is_leader (lp));
+
+  for (lwp_info *other : all_lwps ())
+    {
+      if (other->ptid.pid () == lp->ptid.pid ()
+	  && !is_leader (other)
+	  && other->resumed
+	  && lwp_status_pending_p (other))
+	return true;
+    }
+
+  return false;
+}
+
+/* Indicate whether LP has a pending event which should be considered
+   for immediate processing.  Does not consider a leader thread's exit
+   event before the non-leader threads have reported their exits.  */
+
+static bool
+has_reportable_pending_event (struct lwp_info *lp)
+{
+  return (lp->resumed && lwp_status_pending_p (lp)
+	  && !(!WIFSTOPPED (lp->status)
+	       && is_leader (lp)
+	       && non_leader_lwp_in_process_with_pending_event (lp)));
+}
+
+/* Count the LWP's that have had reportable events.  */
 
 static int
 count_events_callback (struct lwp_info *lp, int *count)
 {
   gdb_assert (count != NULL);
 
-  /* Select only resumed LWPs that have an event pending.  */
-  if (lp->resumed && lwp_status_pending_p (lp))
+  /* Select only resumed LWPs that have a reportable event
+     pending.  */
+  if (has_reportable_pending_event (lp))
     (*count)++;
 
   return 0;
@@ -2526,8 +2561,9 @@ select_event_lwp_callback (struct lwp_info *lp, int *selector)
 {
   gdb_assert (selector != NULL);
 
-  /* Select only resumed LWPs that have an event pending.  */
-  if (lp->resumed && lwp_status_pending_p (lp))
+  /* Select only resumed LWPs that have a reportable event
+     pending.  */
+  if (has_reportable_pending_event (lp))
     if ((*selector)-- == 0)
       return 1;
 
-- 
2.26.2


  parent reply	other threads:[~2022-03-03 14:40 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-03 14:40 [PATCH 00/11] Linux: Fix issues around thread group leader exits Pedro Alves
2022-03-03 14:40 ` [PATCH 01/11] Fix gdbserver/linux target_waitstatus logging assert Pedro Alves
2022-03-03 14:40 ` [PATCH 02/11] Fix gdb.threads/clone-new-thread-event.exp race Pedro Alves
2022-03-03 14:40 ` [PATCH 03/11] Fix gdb.threads/current-lwp-dead.exp race Pedro Alves
2022-03-03 14:40 ` [PATCH 04/11] gdb: Reorganize linux_nat_filter_event Pedro Alves
2022-03-03 14:40 ` [PATCH 05/11] gdbserver: Reorganize linux_process_target::filter_event Pedro Alves
2022-03-03 14:40 ` [PATCH 06/11] gdbserver: Reindent check_zombie_leaders Pedro Alves
2022-03-03 14:40 ` [PATCH 07/11] Re-add zombie leader on exit, gdb/linux Pedro Alves
2022-03-07 20:08   ` Simon Marchi
2022-03-07 20:27     ` Pedro Alves
2022-03-07 20:31       ` Simon Marchi
2022-03-09 14:37         ` Pedro Alves
2022-03-03 14:40 ` [PATCH 08/11] Re-add zombie leader on exit, gdbserver/linux Pedro Alves
2022-03-03 14:40 ` Pedro Alves [this message]
2022-03-07 20:24   ` [PATCH 09/11] Ensure EXIT is last event, gdb/linux Simon Marchi
2022-03-09  0:21     ` Lancelot SIX
2022-03-09 14:45       ` Pedro Alves
2022-03-09 22:29         ` Lancelot SIX
2022-03-10 11:46           ` Pedro Alves
2022-03-03 14:40 ` [PATCH 10/11] Ensure EXIT is last event, gdbserver/linux Pedro Alves
2022-03-03 14:40 ` [PATCH 11/11] Process exit status is leader exit status testcase Pedro Alves
2023-06-22 11:28   ` Ilya Leoshkevich
2023-06-22 13:07     ` Tom de Vries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220303144020.3601082-10-pedro@palves.net \
    --to=pedro@palves.net \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).