Hi!

A few minutes after pushing the GetErrorMode patch [1] I found out that
Win7 and 2k8R2 have added the functions Get/SetThreadErrorMode which
addresses this concern raised in [2]:

> That said, a potential race is still there since the error mode is
> process global and we are trying to save the current error mode and
> restore it afterwards, some other thread might do the same and we'd get
> into an inconsistent state if unlucky. There's no way to deal with that
> though, short of adding some locking mechanism and requiring client code
> to follow our lead.

This patch makes use of the new functions if they are available and
falls back to the previous behavior if they are not (i.e. use GetErrorMode
if it is available, otherwise simulate it with SetErrorMode).

I have been quite busy lately, so I didn't have time to test it properly
for a number of weeks which is why the commit is oldish. But now I have
done testing (on 2k8R2 with Get/SetThreadErrorMode, on (old) Wine with
GetErrorMode and on XP without GetErrorMode) and it behaves as desired.
Recent Wine versions implements Get/SetThreadErrorMode so that was why
I tested with an old Wine version from before the addition of those APIs.

Cheers,
Peter

[1] http://lists.gnu.org/archive/html/libtool-patches/2010-01/msg00004.html
[2] http://lists.gnu.org/archive/html/libtool-patches/2010-01/msg00037.html


2010-01-21  Peter Rosin  <p...@lysator.liu.se>

        Use Get/SetThreadErrorMode if they are available.
        * libltdl/loaders/loadlibrary.c (wrap_geterrormode): Replaced...
        (wrap_getthreaderrormode): ...by this function that checks
        first for GetThreadErrorMode, then GetErrorMode and makes use
        of either of those or...
        (fallback_getthreaderrormode): ...else falls back to this
        replacement function that implements the old workaround, which
        was previously implemented in...
        (fallback_geterrormode): ...this now renamed function.
        (geterrormode): Replaced...
        (getthreaderrormode): ...by this function pointer that points
        at either of wrap_getthreaderrormode, GetThreadErrorMode,
        GetErrorMode or fallback_getthreaderrormode.
        (wrap_setthreaderrormode): New function that checks if
        SetThreadErrorMode is supported by the system and makes use of
        it if it is.
        (fallback_setthreaderrormode): New function that is used
        otherwise that implements the old version using SetErrorMode.
        (setthreaderrormode): New function pointer that points at
        either of wrap_setthreaderrormode, SetThreadErrorMode or
        fallback_setthreaderrormode.
        (vm_open): Adjust to the above.

--
They are in the crowd with the answer before the question.
> Why do you dislike Jeopardy?
diff --git a/libltdl/loaders/loadlibrary.c b/libltdl/loaders/loadlibrary.c
index 139851a..ad3470a 100644
--- a/libltdl/loaders/loadlibrary.c
+++ b/libltdl/loaders/loadlibrary.c
@@ -104,12 +104,16 @@ get_vtable (lt_user_data loader_data)
 #define LOADLIB_SETERROR(errcode) LOADLIB__SETERROR (LT__STRERROR (errcode))
 
 static const char *loadlibraryerror (const char *default_errmsg);
-static UINT WINAPI wrap_geterrormode (void);
-static UINT WINAPI fallback_geterrormode (void);
+static DWORD WINAPI wrap_getthreaderrormode (void);
+static DWORD WINAPI fallback_getthreaderrormode (void);
+static BOOL WINAPI wrap_setthreaderrormode (DWORD mode, DWORD *oldmode);
+static BOOL WINAPI fallback_setthreaderrormode (DWORD mode, DWORD *oldmode);
 
-typedef UINT (WINAPI geterrormode_type) (void);
+typedef DWORD (WINAPI getthreaderrormode_type) (void);
+typedef BOOL (WINAPI setthreaderrormode_type) (DWORD, DWORD *);
 
-static geterrormode_type *geterrormode = wrap_geterrormode;
+static getthreaderrormode_type *getthreaderrormode = wrap_getthreaderrormode;
+static setthreaderrormode_type *setthreaderrormode = wrap_setthreaderrormode;
 static char *error_message = 0;
 
 
@@ -187,13 +191,13 @@ vm_open (lt_user_data LT__UNUSED loader_data, const char 
*filename,
 
   {
     /* Silence dialog from LoadLibrary on some failures. */
-    UINT errormode = geterrormode ();
-    SetErrorMode (errormode | SEM_FAILCRITICALERRORS);
+    DWORD errormode = getthreaderrormode ();
+    setthreaderrormode (errormode | SEM_FAILCRITICALERRORS, NULL);
 
     module = LoadLibrary (wpath);
 
     /* Restore the error mode. */
-    SetErrorMode (errormode);
+    setthreaderrormode (errormode, NULL);
   }
 
   /* libltdl expects this function to fail if it is unable
@@ -298,28 +302,61 @@ loadlibraryerror (const char *default_errmsg)
   return len ? error_message : default_errmsg;
 }
 
-/* A function called through the geterrormode variable which checks
-   if the system supports GetErrorMode and arranges for it or a
-   fallback implementation to be called directly in the future. The
-   selected version is then called. */
-static UINT WINAPI
-wrap_geterrormode (void)
+/* A function called through the getthreaderrormode variable which checks
+   if the system supports GetThreadErrorMode (or GetErrorMode) and arranges
+   for it or a fallback implementation to be called directly in the future.
+   The selected version is then called. */
+static DWORD WINAPI
+wrap_getthreaderrormode (void)
 {
   HMODULE kernel32 = GetModuleHandleA ("kernel32.dll");
-  geterrormode = (geterrormode_type *) GetProcAddress (kernel32,
-                                                       "GetErrorMode");
-  if (!geterrormode)
-    geterrormode = fallback_geterrormode;
-  return geterrormode ();
+  getthreaderrormode = (getthreaderrormode_type *) GetProcAddress (
+    kernel32, "GetThreadErrorMode");
+  if (!getthreaderrormode)
+    getthreaderrormode = (getthreaderrormode_type *) GetProcAddress (
+      kernel32, "GetErrorMode");
+  if (!getthreaderrormode)
+    getthreaderrormode = fallback_getthreaderrormode;
+  return getthreaderrormode ();
 }
 
-/* A function called through the geterrormode variable for cases
-   where the system does not support GetErrorMode */
-static UINT WINAPI
-fallback_geterrormode (void)
+/* A function called through the getthreaderrormode variable for cases
+   where the system does not support GetThreadErrorMode or GetErrorMode */
+static DWORD WINAPI
+fallback_getthreaderrormode (void)
 {
   /* Prior to Windows Vista, the only way to get the current error
      mode was to set a new one. In our case, we are setting a new
-     error mode right after "getting" it, so that's fairly ok. */
-  return SetErrorMode (SEM_FAILCRITICALERRORS);
+     error mode right after "getting" it while ignoring the error
+     mode in effect when setting the new error mode, so that's
+     fairly ok. */
+  return (DWORD) SetErrorMode (SEM_FAILCRITICALERRORS);
+}
+
+/* A function called through the setthreaderrormode variable which checks
+   if the system supports SetThreadErrorMode and arranges for it or a
+   fallback implementation to be called directly in the future.
+   The selected version is then called. */
+static BOOL WINAPI
+wrap_setthreaderrormode (DWORD mode, DWORD *oldmode)
+{
+  HMODULE kernel32 = GetModuleHandleA ("kernel32.dll");
+  setthreaderrormode = (setthreaderrormode_type *) GetProcAddress (
+    kernel32, "SetThreadErrorMode");
+  if (!setthreaderrormode)
+    setthreaderrormode = fallback_setthreaderrormode;
+  return setthreaderrormode (mode, old_mode);
+}
+
+/* A function called through the setthreaderrormode variable for cases
+   where the system does not support SetThreadErrorMode. */
+static BOOL WINAPI
+fallback_setthreaderrormode (DWORD mode, DWORD *oldmode)
+{
+  /* Prior to Windows 7, there was no way to set the thread local error
+     mode, so set the process global error mode instead. */
+  DWORD old = (DWORD) SetErrorMode (mode);
+  if (oldmode)
+    *oldmode = old;
+  return TRUE;
 }

Reply via email to