Hello,

my buildfarm animal woodlouse (Visual Studio 2013 on Windows 7) stopped working correctly some months ago. After Tom kindly prodded me into fixing it, I noticed that I had configured it to skip the ecpg-check step because one of the tests in the "thread" section (not always the same) nearly always failed.

I had set it up in circa 2015, so I reenabled the step to see whether anything had changed since, and it started failing again.

Through some trial and error, and with the help of Microsoft's Application Verifier tool, I found what I think is the cause: It looks like the VS 2013 CRT's setlocale() function is not entirely thread-safe; the heap debugging options make it crash when manipulating per-thread locale state, and according to the comments surrounding that spot in the CRT source, the developers were aware the code is fragile.

I crudely hacked a critical section around the setlocale() call in pgwin32_setlocale(). After this change, the test does not crash, while without it, it crashes every time.

If there is interest in fixing this issue that is apparently limited to VS 2013, I will try and produce a proper patch. I notice, however, that there is a pthread compatibility layer available that I have no experience with at all, and I assume using it is preferred over straight Win32?

My hack is attached; please feel free to tell me I'm on the wrong track.
To build correctly, it requires defining _WIN32_WINNT to be 0x600 or above (and using an SDK that knows about InitOnceExecuteOnce()).

--
Christian
diff --git a/src/port/win32setlocale.c b/src/port/win32setlocale.c
index cbf109836b..278d836b4d 100644
--- a/src/port/win32setlocale.c
+++ b/src/port/win32setlocale.c
@@ -164,19 +164,33 @@ map_locale(const struct locale_map * map, const char 
*locale)
        return locale;
 }
 
+static CRITICAL_SECTION setlocale_cs;
+static INIT_ONCE init_once;
+static BOOL CALLBACK init_setlocale_cs(PINIT_ONCE pInitOnce, PVOID pParam, 
PVOID pCtx)
+{
+       InitializeCriticalSection((PCRITICAL_SECTION)pParam);
+       return TRUE;
+}
+
 char *
 pgwin32_setlocale(int category, const char *locale)
 {
        const char *argument;
        char       *result;
 
+       if (InitOnceExecuteOnce(&init_once, init_setlocale_cs, 
(PVOID)&setlocale_cs, NULL) == 0) {
+               abort();
+       }
+
        if (locale == NULL)
                argument = NULL;
        else
                argument = map_locale(locale_map_argument, locale);
 
        /* Call the real setlocale() function */
+       EnterCriticalSection(&setlocale_cs);
        result = setlocale(category, argument);
+       LeaveCriticalSection(&setlocale_cs);
 
        /*
         * setlocale() is specified to return a "char *" that the caller is
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to