[PATCH v2 0/1] MR6644: Draft: msvcrt: Initialize locale data in new threads.

[Bernhard Übelacker]

This attempts to initialize the thread data more like in the mainthread,
which does in msvcrt_init_locale a call to _setmbcp, which finally
fills the cached_locale member of the thread data.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57250

--
  v2: msvcrt: Initialize locale data in new threads.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6644

[PATCH v2 1/1] msvcrt: Initialize locale data in new threads.

[Bernhard Übelacker]

From: Bernhard Übelacker <bernhardu(a)mailbox.org>

This attempts to initialize the thread data more like in the mainthread,
which does in msvcrt_init_locale a call to _setmbcp, which finally
fills the cached_locale member of the thread data.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57250
---
 dlls/msvcrt/tests/locale.c | 25 +++++++++++++++++++++++++
 dlls/msvcrt/thread.c       |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/dlls/msvcrt/tests/locale.c b/dlls/msvcrt/tests/locale.c
index cb56fe39754..70496b9efcf 100644
--- a/dlls/msvcrt/tests/locale.c
+++ b/dlls/msvcrt/tests/locale.c
@@ -19,6 +19,7 @@
  */
 
 #include <locale.h>
+#include <process.h>
 
 #include "wine/test.h"
 #include "winnls.h"
@@ -845,6 +846,29 @@ static void test__wcsicmp_l(void)
     }
 }
 
+static void __cdecl test_thread_setlocale_func(void * arg __attribute__((unused)))
+{
+    char *ret;
+
+    ret = setlocale(LC_ALL, NULL);
+    ok(!strcmp(ret, "C"), "expected ret=C, but received ret=%s\n", ret);
+
+    ret = setlocale(LC_ALL, "");
+    ok(strcmp(ret, "Invariant Language_Invariant Country.0"), "expected something different, but received ret=%s\n", ret);
+
+    _endthread();
+}
+
+static void test_thread_setlocale(void)
+{
+    HANDLE hThread;
+
+    hThread = (HANDLE)_beginthread(test_thread_setlocale_func, 0, NULL);
+    ok(hThread != INVALID_HANDLE_VALUE, "_beginthread failed (%d)\n", errno);
+    WaitForSingleObject(hThread, 5000);
+    CloseHandle(hThread);
+}
+
 START_TEST(locale)
 {
     init();
@@ -854,4 +878,5 @@ START_TEST(locale)
     test__Gettnames();
     test___mb_cur_max_func();
     test__wcsicmp_l();
+    test_thread_setlocale();
 }
diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c
index 6aad9a22d47..bd0214029bf 100644
--- a/dlls/msvcrt/thread.c
+++ b/dlls/msvcrt/thread.c
@@ -18,6 +18,7 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  */
 #include <process.h>
+#include <mbctype.h>
 #include "msvcrt.h"
 #include "wine/debug.h"
 
@@ -60,6 +61,7 @@ thread_data_t *CDECL msvcrt_get_thread_data(void)
 #if _MSVCR_VER >= 140
         ptr->module = NULL;
 #endif
+        _setmbcp(_MB_CP_ANSI);
     }
     SetLastError( err );
     return ptr;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6644

[Bernhard Übelacker]

v2:
- Fix build-clang error in thread function: omitting the parameter name in a function definition is a C2x extension

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6644#note_84572

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcrt/tests/locale.c:
> }
>  }
>
> +static void __cdecl test_thread_setlocale_func(void * arg __attribute__((unused)))
> +{
> +    char *ret;
> +
> +    ret = setlocale(LC_ALL, NULL);
> +    ok(!strcmp(ret, "C"), "expected ret=C, but received ret=%s\n", ret);
> +
> +    ret = setlocale(LC_ALL, "");
> +    ok(strcmp(ret, "Invariant Language_Invariant Country.0"), "expected something different, but received ret=%s\n", ret);

Could you please change the error string to something like: `"expected valid locale\n"`? It's already known what's the value of ret if the test fails.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6644#note_84573

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcrt/tests/locale.c:
> }
>  }
>
> +static void __cdecl test_thread_setlocale_func(void * arg __attribute__((unused)))
> +{
> +    char *ret;
> +
> +    ret = setlocale(LC_ALL, NULL);
> +    ok(!strcmp(ret, "C"), "expected ret=C, but received ret=%s\n", ret);
> +
> +    ret = setlocale(LC_ALL, "");
> +    ok(strcmp(ret, "Invariant Language_Invariant Country.0"), "expected something different, but received ret=%s\n", ret);
> +
> +    _endthread();

There's no need to call `_endthread()` here.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6644#note_84574

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcrt/thread.c:
> #if _MSVCR_VER >= 140
>          ptr->module = NULL;
>  #endif
> +        _setmbcp(_MB_CP_ANSI);
> You can't call _setmbcp() here - it might modify other thread mbcp when global (non per-thread) settings are used. The bug is caused by using uninitialized locale cache. The simplest solution is to set:
ptr->cached_locale[0] = 'C';
ptr->cached_locale[1] = 0;

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6644#note_84575

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcrt/tests/locale.c:
> +    char *ret;
> +
> +    ret = setlocale(LC_ALL, NULL);
> +    ok(!strcmp(ret, "C"), "expected ret=C, but received ret=%s\n", ret);
> +
> +    ret = setlocale(LC_ALL, "");
> +    ok(strcmp(ret, "Invariant Language_Invariant Country.0"), "expected something different, but received ret=%s\n", ret);
> +
> +    _endthread();
> +}
> +
> +static void test_thread_setlocale(void)
> +{
> +    HANDLE hThread;
> +
> +    hThread = (HANDLE)_beginthread(test_thread_setlocale_func, 0, NULL);
> Please use `_beginthreadex` here. Otherwise the thread handle will be closed when the thread finishes.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6644#note_84576

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcrt/tests/locale.c:
> }
>  }
>
> +static void __cdecl test_thread_setlocale_func(void * arg __attribute__((unused)))

__attribute__((unused)) is probably not portable, please remove it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6644#note_84577