[PATCH] Ignore the last EXIT_THREAD_DEBUG_EVENT on Windows

Tom Tromey <[email protected]>
Newsgroups gmane.comp.gdb.patches
Message-ID <[email protected]>
The Windows non-stop series caused some regressions in the internal
AdaCore test suite.  I filed this as PR tdep/34195.  I finally found
some time to look into the problem, and this patch is the result.

The symptom of the problem is that some fairly ordinary tests -- like
a test to simply run a do-nothing program to completion -- would fail
because gdb would print "No unwaited-for children left."

I bisected the problem to commit ae2f226d ("Windows gdb: Add non-stop
support").

Instrumenting the test suite to enable logging showed something
strange: in the failing case, we'd see and EXIT_THREAD_DEBUG_EVENT for
each thread, but never an EXIT_PROCESS_DEBUG_EVENT.

And, since gdb didn't see or handle an EXIT_PROCESS_DEBUG_EVENT, the
process would remain live -- one of the internal AdaCore tests checks
this (via some "ps" equivalent), causing a separate failure.

After a lot of experimentation I came up with this patch.  It changes
gdb to ignore the final EXIT_THREAD_DEBUG_EVENT, waiting instead for
an EXIT_PROCESS_DEBUG_EVENT to do the cleanup.  This fixed the
regressions in my testing.

I still don't fully understand this bug, though:

1. It only happened under load, I was never able to reproduce it by
   running a single test case.  It's worth noting, though, that I ran
   the test suite against gdb 17 on the same Windows instance, and
   that always worked flawlessly -- so it seems like it's definitely a
   gdb bug and not an OS bug.

2. Windows supposedly shouldn't even issue an EXIT_THREAD_DEBUG_EVENT
   for the final thread, only EXIT_PROCESS_DEBUG_EVENT.  At least
   IIUC.  So, the fact that we're even seeing this seems mysterious.
   However it's worth noting that the patch only affects this case, so
   if Windows does not emit that final thread-exit event, everything
   should still work fine.

I've marked this bug as blocking gdb 18 because it is a regression.
If approved I plan to apply the patch to that branch as well.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34195
---
 gdb/windows-nat.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index def8fa606f4..0e958ea8186 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1570,6 +1570,32 @@ windows_nat_target::get_windows_debug_event
 
     case EXIT_THREAD_DEBUG_EVENT:
       {
+	/* If we see the last thread-exited event, and then we go
+	   ahead and delete the thread, this might cause gdb to stop
+	   issuing calls to ContinueDebugEvent.  See PR dept/34195.
+	   This has two bad effects.
+
+	   First, the EXIT_PROCESS_DEBUG_EVENT is never handled, so
+	   the process doesn't truly exit -- something that can be
+	   detected by examining the running processes on the system
+	   (i.e., using the equivalent of "ps").
+
+	   Second, gdb will tell the user "No unwaited-for children
+	   left", which is not really something users do or should
+	   understand.
+
+	   This stanza works around this problem: we treat the exit of
+	   the last remaining thread as a spurious event, causing gdb
+	   to call ContinueDebugEvent; the thread exit is then handled
+	   by the EXIT_PROCESS_DEBUG_EVENT.  */
+	inferior *inf = find_inferior_pid (this, current_event->dwProcessId);
+	auto rng = inf->non_exited_threads ();
+	if (std::distance (rng.begin (), rng.end ()) == 1)
+	  {
+	    ourstatus->set_spurious ();
+	    return null_ptid;
+	  }
+
 	ourstatus->set_thread_exited
 	  (current_event->u.ExitThread.dwExitCode);
 	thread_id = current_event->dwThreadId;

base-commit: a3a56294a7c5e5898309c04cc3b1e490cca76ef7
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.