Make pgwin32_putenv() follow DLL loading and unloading.

Until now, the first putenv() call of a given postgres.exe process would
cache the set of loaded CRTs.  If a CRT unloaded after that call, the
next putenv() would crash.  That risk was largely theoretical, because
the first putenv() precedes all PostgreSQL-initiated module loading.
However, this might explain bad interactions with antivirus and other
software that injects threads asynchronously.  If an additional CRT
loaded after the first putenv(), pgwin32_putenv() would not discover it.
That CRT would have all environment changes predating its load, but it
would not receive later PostgreSQL-initiated changes.  An additional CRT
loading concurrently with the first putenv() might miss that change in
addition to missing later changes.  Fix all those problems.  This
removes the cache mechanism from pgwin32_putenv(); the cost, less than
100 μs per backend startup, is negligible.

No resulting misbehavior was known to be user-visible given the core
distribution alone, but one can readily construct an affected extension
module.  No back-patch given the lack of complaints and the potential
for behavior changes in non-PostgreSQL code running in the backend.

Christian Ullrich, reviewed by Michael Paquier.

diff --git a/src/port/win32env.c b/src/port/win32env.c
index 4e4cb5e294dca4e40b9cb2a2e8eb41e319081f07..a491497152cac5244d4421989e34796a29d21d94 100644 (file)
--- a/src/port/win32env.c
+++ b/src/port/win32env.c
@@ -21,126 +21,37 @@ pgwin32_putenv(const char *envval)
 {
    char       *envcpy;
    char       *cp;
-
-   /*
-    * Each CRT has its own _putenv() symbol and copy of the environment.
-    * Update the environment in each CRT module currently loaded, so every
-    * third-party library sees this change regardless of the CRT it links
-    * against.
-    */
 #ifdef _MSC_VER
    typedef int (_cdecl * PUTENVPROC) (const char *);
-   static struct
-   {
-       char       *modulename;
-       HMODULE     hmodule;
-       PUTENVPROC  putenvFunc;
-   }           rtmodules[] =
-   {
-       {
-           "msvcrt", NULL, NULL
-       },                      /* Visual Studio 6.0 / MinGW */
-       {
-           "msvcrtd", NULL, NULL
-       },
-       {
-           "msvcr70", NULL, NULL
-       },                      /* Visual Studio 2002 */
-       {
-           "msvcr70d", NULL, NULL
-       },
-       {
-           "msvcr71", NULL, NULL
-       },                      /* Visual Studio 2003 */
-       {
-           "msvcr71d", NULL, NULL
-       },
-       {
-           "msvcr80", NULL, NULL
-       },                      /* Visual Studio 2005 */
-       {
-           "msvcr80d", NULL, NULL
-       },
-       {
-           "msvcr90", NULL, NULL
-       },                      /* Visual Studio 2008 */
-       {
-           "msvcr90d", NULL, NULL
-       },
-       {
-           "msvcr100", NULL, NULL
-       },                      /* Visual Studio 2010 */
-       {
-           "msvcr100d", NULL, NULL
-       },
-       {
-           "msvcr110", NULL, NULL
-       },                      /* Visual Studio 2012 */
-       {
-           "msvcr110d", NULL, NULL
-       },
-       {
-           "msvcr120", NULL, NULL
-       },                      /* Visual Studio 2013 */
-       {
-           "msvcr120d", NULL, NULL
-       },
-       {
-           "ucrtbase", NULL, NULL
-       },                      /* Visual Studio 2015 and later */
-       {
-           "ucrtbased", NULL, NULL
-       },
-       {
-           NULL, NULL, NULL
-       }
+   static const char *const modulenames[] = {
+       "msvcrt",               /* Visual Studio 6.0 / MinGW */
+       "msvcrtd",
+       "msvcr70",              /* Visual Studio 2002 */
+       "msvcr70d",
+       "msvcr71",              /* Visual Studio 2003 */
+       "msvcr71d",
+       "msvcr80",              /* Visual Studio 2005 */
+       "msvcr80d",
+       "msvcr90",              /* Visual Studio 2008 */
+       "msvcr90d",
+       "msvcr100",             /* Visual Studio 2010 */
+       "msvcr100d",
+       "msvcr110",             /* Visual Studio 2012 */
+       "msvcr110d",
+       "msvcr120",             /* Visual Studio 2013 */
+       "msvcr120d",
+       "ucrtbase",             /* Visual Studio 2015 and later */
+       "ucrtbased",
+       NULL
    };
    int         i;
-
-   for (i = 0; rtmodules[i].modulename; i++)
-   {
-       if (rtmodules[i].putenvFunc == NULL)
-       {
-           if (rtmodules[i].hmodule == NULL)
-           {
-               /* Not attempted before, so try to find this DLL */
-               rtmodules[i].hmodule = GetModuleHandle(rtmodules[i].modulename);
-               if (rtmodules[i].hmodule == NULL)
-               {
-                   /*
-                    * Set to INVALID_HANDLE_VALUE so we know we have tried
-                    * this one before, and won't try again.
-                    */
-                   rtmodules[i].hmodule = INVALID_HANDLE_VALUE;
-                   continue;
-               }
-               else
-               {
-                   rtmodules[i].putenvFunc = (PUTENVPROC) GetProcAddress(rtmodules[i].hmodule, "_putenv");
-                   if (rtmodules[i].putenvFunc == NULL)
-                   {
-                       rtmodules[i].hmodule = INVALID_HANDLE_VALUE;
-                       continue;
-                   }
-               }
-           }
-           else
-           {
-               /*
-                * Module loaded, but we did not find the function last time.
-                * We're not going to find it this time either...
-                */
-               continue;
-           }
-       }
-       /* At this point, putenvFunc is set or we have exited the loop */
-       rtmodules[i].putenvFunc(envval);
-   }
 #endif   /* _MSC_VER */
 
    /*
     * Update process environment, making this change visible to child
-    * processes and to CRTs initializing in the future.
+    * processes and to CRTs initializing in the future.  Do this before the
+    * _putenv() loop, for the benefit of any CRT that initializes during this
+    * pgwin32_putenv() execution, after the loop checks that CRT.
     *
     * Need a copy of the string so we can modify it.
     */
@@ -170,6 +81,31 @@ pgwin32_putenv(const char *envval)
    }
    free(envcpy);
 
+   /*
+    * Each CRT has its own _putenv() symbol and copy of the environment.
+    * Update the environment in each CRT module currently loaded, so every
+    * third-party library sees this change regardless of the CRT it links
+    * against.  Addresses within these modules may become invalid the moment
+    * we call FreeLibrary(), so don't cache them.
+    */
+#ifdef _MSC_VER
+   for (i = 0; modulenames[i]; i++)
+   {
+       HMODULE     hmodule = NULL;
+       BOOL        res = GetModuleHandleEx(0, modulenames[i], &hmodule);
+
+       if (res != 0 && hmodule != NULL)
+       {
+           PUTENVPROC  putenvFunc;
+
+           putenvFunc = (PUTENVPROC) GetProcAddress(hmodule, "_putenv");
+           if (putenvFunc)
+               putenvFunc(envval);
+           FreeLibrary(hmodule);
+       }
+   }
+#endif   /* _MSC_VER */
+
    /* Finally, update our "own" cache */
    return _putenv(envval);
 }