From 3444e1db18bcd1b9f7425a31895298c3864d294c Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Wed, 2 Jan 2019 17:29:48 -0600 Subject: [PATCH] Fix unsafe locale usage in `wcstod_l` fallback Using `setlocale` is both not thread-safe and not correct, as a) The global locale is usually stored in static storage, so simultaneous calls to `setlocale` can result in corruption, and b) `setlocale` changes the locale for the entire application, not just the calling thread. This means that even if we wrapped the `wcstod_l` in a mutex to prevent the previous point, the results would still be incorrect because this would incorrectly influence the results of locale-aware functions executed in other threads while this thread is executing. The previous comment mentioned that `uselocale` hadn't worked. I'm not sure what the failing implementation looked like, but `uselocale` can be tricky. The committed implementation passes the tests for me under Linux and FreeBSD. --- src/fallback.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/fallback.cpp b/src/fallback.cpp index a0db093a9..bcfc4d926 100644 --- a/src/fallback.cpp +++ b/src/fallback.cpp @@ -394,13 +394,13 @@ int flock(int fd, int op) { // For platforms without wcstod_l C extension, wrap wcstod after changing the // thread-specific locale. double fish_compat::wcstod_l(const wchar_t *enptr, wchar_t **endptr, locale_t loc) { - char *saved_locale = strdup(setlocale(LC_NUMERIC, NULL)); - // Yes, this is hardcoded to use the "C" locale. - // That's the only thing we need, and uselocale(loc) broke in my testing. - setlocale(LC_NUMERIC, "C"); + // Create and use a new, thread-specific locale + locale_t locale = newlocale(LC_NUMERIC, "C", nullptr); + locale_t prev_locale = uselocale(locale); double ret = wcstod(enptr, endptr); - setlocale(LC_NUMERIC, saved_locale); - free(saved_locale); + // Restore the old locale before freeing the locale we created and are still using + uselocale(prev_locale); + freelocale(locale); return ret; } #endif // defined(wcstod_l)