[PATCH] Cygwin: Remove waitloop argument from try_to_debug()

Jon Turney jon.turney@dronecode.org.uk
Sat Aug 29 14:43:32 GMT 2020


Currently, when using CYGWIN='error_start=dumper', the core dump written
in response to an exception is non-deterministic, as the faulting
process isn't stopped while the dumper is started (it even seems
possible in theory that the faulting process could have exited before
the dumper process attaches).

Remove the waitloop argument, only used in this case, so the faulting
process busy-waits until the dump starts.

Code archeology to determine why the code is this way didn't really turn
up any answers, but this seems a low-risk change, as this only changes
the behaviour when:

 - a debugger isn't already attached
 - an error_start is specified in CYGWIN env var
 - an exception has occured which will be translated to a signal

Future work: This probably can be further simplified to make it
completely synchronous by waiting for the dumper process to exit. This
would avoid the race condition of the dumper attaching and detaching
before we get around to checking for that (which we try to work around
by juggling thread priorities), and the failure state where the dumper
doesn't attach and we spin indefinitely.
---
 winsup/cygwin/exceptions.cc | 8 ++------
 winsup/cygwin/winsup.h      | 2 +-
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index bb7704f94..3ed2db1eb 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -461,10 +461,8 @@ cygwin_stackdump ()
   exc.dumpstack ();
 }
 
-#define TIME_TO_WAIT_FOR_DEBUGGER 10000
-
 extern "C" int
-try_to_debug (bool waitloop)
+try_to_debug ()
 {
   if (!debugger_command)
     return 0;
@@ -537,8 +535,6 @@ try_to_debug (bool waitloop)
     system_printf ("Failed to start debugger, %E");
   else
     {
-      if (!waitloop)
-	return dbg;
       SetThreadPriority (GetCurrentThread (), THREAD_PRIORITY_IDLE);
       while (!being_debugged ())
 	Sleep (1);
@@ -812,7 +808,7 @@ exception::handle (EXCEPTION_RECORD *e, exception_list *frame, CONTEXT *in,
   if (exit_state >= ES_SIGNAL_EXIT
       && (NTSTATUS) e->ExceptionCode != STATUS_CONTROL_C_EXIT)
     api_fatal ("Exception during process exit");
-  else if (!try_to_debug (0))
+  else if (!try_to_debug ())
     rtl_unwind (frame, e);
   else
     {
diff --git a/winsup/cygwin/winsup.h b/winsup/cygwin/winsup.h
index 79844cb87..0ffd8c5af 100644
--- a/winsup/cygwin/winsup.h
+++ b/winsup/cygwin/winsup.h
@@ -190,7 +190,7 @@ void close_all_files (bool = false);
 
 /* debug_on_trap support. see exceptions.cc:try_to_debug() */
 extern "C" void error_start_init (const char*);
-extern "C" int try_to_debug (bool waitloop = 1);
+extern "C" int try_to_debug ();
 
 void ld_preload ();
 void fixup_hooks_after_fork ();
-- 
2.28.0



More information about the Cygwin-patches mailing list