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
{
--
Problem reports: https://cygwin.com/problems.html
FAQ: https://cygwin.com/faq/
Documentation: https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
