Restore SEM_FAILCRITICALERRORS [was: Aren't Windows System Error popups meant to be disabled in Cygwin?]

Corinna Vinschen corinna-cygwin@cygwin.com
Fri Feb 2 17:22:38 GMT 2024


On Feb  2 14:56, David Allsopp via Cygwin wrote:
> On Fri, 2 Feb 2024 at 14:18, Corinna Vinschen via Cygwin wrote:
> > On Feb  2 13:35, David Allsopp via Cygwin wrote:
> > > Not really suggesting it be done this way (it feels more complicated
> > > than just reverting the change), but in some ways perhaps Cygwin
> > > should be using GetErrorMode on startup and instead of not inheriting
> > > it, ensuring that it sets whatever it received? i.e. just before the
> > > call to CreateProcess for a non-Cygwin binary, Cygwin restores the
> > > error mode (for that thread only) to the value read at startup, calls
> > > CreateProcess and then sets the error mode back.
> >
> > This sounds like a good ide, but...
> >
> > Is it actually a safe bet that the error mode set by SetThreadErrorMode
> > is then propagated as process error mode to the child process?
> >
> > I have to ask that because Microsoft conveniently forgot to document
> > this scenario in the MSDN docs.
> 
> :o) Never knowingly clear, are they! It would seem to be the intent of
> SetThreadErrorMode that it would behave that way but who knows.
> 
> Happy to set up a quick experiment to check that it does work (i.e.
> the invoked process has GetErrorMode set as expected) and that there's
> no possible race between two threads in the calling process with
> differing values (i.e. that having SEM_FAILCRITICALERRORS in one
> thread and not in another behaves as expected. If it does appear to
> work consistently, would you be willing to go down this route? Happy
> to do the patch, although it'd be very helpful to have a couple of
> pointers: I'm guessing the value would want to be captured just before
> the one place where SetErrorMode is already called, but in which
> structure should it then be stashed away to be reused in spawn?

Wanna try this?  It ignores the case of starting a process
under another user account for now, but that can be added easily
if this proves to work as expected.

diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index a40129c22232..f1017e69b6b2 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -718,7 +718,8 @@ dll_crt0_0 ()
   init_windows_system_directory ();
   initial_env ();
 
-  SetErrorMode (SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX);
+  orig_proc_error_mode = SetErrorMode (SEM_FAILCRITICALERRORS
+				       | SEM_NOGPFAULTERRORBOX);
 
   lock_process::init ();
   user_data->impure_ptr = _impure_ptr;
diff --git a/winsup/cygwin/globals.cc b/winsup/cygwin/globals.cc
index 885ada85e7b8..d2861ba98b42 100644
--- a/winsup/cygwin/globals.cc
+++ b/winsup/cygwin/globals.cc
@@ -28,6 +28,7 @@ PWCHAR windows_directory = windows_directory_buf + 4;
 UINT windows_directory_length;
 UNICODE_STRING windows_directory_path;
 WCHAR global_progname[NT_MAX_PATH];
+UINT orig_proc_error_mode;
 
 /* program exit the program */
 
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 8a2db5cf72e2..df83f25d13c6 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -648,6 +648,10 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	      && !::cygheap->user.groups.issetgroups ()
 	      && !::cygheap->user.setuid_to_restricted))
 	{
+	  UINT orig_thread_error_mode = SEM_FAILCRITICALERRORS
+					| SEM_NOGPFAULTERRORBOX;
+	  if (!iscygwin ())
+	    SetThreadErrorMode (orig_proc_error_mode, &orig_thread_error_mode);
 	  rc = CreateProcessW (runpath,		/* image name w/ full path */
 			       cmd.wcs (wcmd),	/* what was passed to exec */
 			       sa,		/* process security attrs */
@@ -658,6 +662,8 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 			       NULL,
 			       &si,
 			       &pi);
+	  if (!iscygwin ())
+	    SetThreadErrorMode (orig_thread_error_mode, NULL);
 	}
       else
 	{


More information about the Cygwin mailing list