XIM doesn't work when LC_CTYPE category is set to C using setlocale.
Signed-off-by: Piotr Caban piotr@codeweavers.com --- configure.ac | 1 + dlls/kernel32/locale.c | 12 +++++++++++- dlls/ntdll/thread.c | 10 ++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-)
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=49652
Your paranoid android.
=== debian9 (32 bit report) ===
ntdll: pipe.c:1557: Test failed: pipe is not signaled pipe.c:1557: Test failed: pipe is not signaled
On Mar 20, 2019, at 2:02 PM, Piotr Caban piotr@codeweavers.com wrote:
XIM doesn't work when LC_CTYPE category is set to C using setlocale.
diff --git a/configure.ac b/configure.ac index d5640ed0e9..4a410f8aea 100644 --- a/configure.ac +++ b/configure.ac @@ -2176,6 +2176,7 @@ AC_CHECK_FUNCS(\ tcdrain \ thr_kill2 \ timegm \
uselocale \
The other lines use a tab and yours uses spaces.
diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c index 9f4a08fdb9..e25cb7848d 100644 --- a/dlls/ntdll/thread.c +++ b/dlls/ntdll/thread.c @@ -24,6 +24,7 @@ #include <assert.h> #include <stdarg.h> #include <limits.h> +#include <locale.h> #include <sys/types.h> #ifdef HAVE_SYS_MMAN_H #include <sys/mman.h> @@ -371,6 +372,15 @@ static void start_thread( struct startup_info *info ) TEB *teb = info->teb; struct ntdll_thread_data *thread_data = (struct ntdll_thread_data *)&teb->GdiTebBatch; struct debug_info debug_info; +#ifdef HAVE_USELOCALE
- locale_t global_loc, c_loc;
- /* Fixes issues when tr_TR.UTF-8 locale is used */
- global_loc = duplocale( LC_GLOBAL_LOCALE );
- c_loc = newlocale( LC_CTYPE_MASK, "C", global_loc );
- if (!c_loc) freelocale( global_loc );
- else if (!uselocale( c_loc )) freelocale( c_loc );
+#endif
It would be better to not duplicate the locale objects for every thread (and leak them). You can use static variables and one-time initialization. And perhaps share them with the ones in kernel32, perhaps via libwine.
-Ken
On 3/20/19 8:43 PM, Ken Thomases wrote:
It would be better to not duplicate the locale objects for every thread (and leak them). You can use static variables and one-time initialization. And perhaps share them with the ones in kernel32, perhaps via libwine.
There's no leak, it's freed by glibc on thread/process destruction.
On 3/20/19 8:52 PM, Piotr Caban wrote:
On 3/20/19 8:43 PM, Ken Thomases wrote:
It would be better to not duplicate the locale objects for every thread (and leak them). You can use static variables and one-time initialization. And perhaps share them with the ones in kernel32, perhaps via libwine.
There's no leak, it's freed by glibc on thread/process destruction.
Sorry, I've checked it wrongly. glibc only frees it for the main thread, it's leaked in other threads.
Thanks, Piotr
On Mar 20, 2019, at 3:04 PM, Piotr Caban piotr.caban@gmail.com wrote:
On 3/20/19 8:52 PM, Piotr Caban wrote:
On 3/20/19 8:43 PM, Ken Thomases wrote:
It would be better to not duplicate the locale objects for every thread (and leak them). You can use static variables and one-time initialization. And perhaps share them with the ones in kernel32, perhaps via libwine.
There's no leak, it's freed by glibc on thread/process destruction.
Sorry, I've checked it wrongly. glibc only frees it for the main thread, it's leaked in other threads.
As it turns out, the macOS implementation works differently than Linux/POSIX. Maybe that's common to BSDs generally, I don't know.
In macOS's C library, the thread locale is freed on thread exit, for all threads. The thread locale is stored in a thread-specific key with a destructor. There is some internal reference counting, but the locale object itself is freed unconditionally so that doesn't help us any. So, my concern about leaking was misplaced, as was my suggestion to avoid duplicating the locale for every thread. But now there's a concern about glibc.
Also, on macOS, newlocale() never modifies nor frees base. The new locale is always newly-created and independent from base and base is left alone. (Internally, it duplicates base to create the new locale and then modifies it as per the mask and locale string.)
So, global_loc is leaked on macOS.
Kind of a mess. :(
-Ken
Hi Ken,
On 3/20/19 10:11 PM, Ken Thomases wrote:
As it turns out, the macOS implementation works differently than Linux/POSIX. Maybe that's common to BSDs generally, I don't know.
In macOS's C library, the thread locale is freed on thread exit, for all threads. The thread locale is stored in a thread-specific key with a destructor. There is some internal reference counting, but the locale object itself is freed unconditionally so that doesn't help us any. So, my concern about leaking was misplaced, as was my suggestion to avoid duplicating the locale for every thread. But now there's a concern about glibc.
Also, on macOS, newlocale() never modifies nor frees base. The new locale is always newly-created and independent from base and base is left alone. (Internally, it duplicates base to create the new locale and then modifies it as per the mask and locale string.)
So, global_loc is leaked on macOS.
It's a shame it was not well standardized. It probably makes the function not suitable for us (unless all the other solutions are even worse).
Kind of a mess. :(I've filled a bug to man-pages project regarding newlocale man page.
According to valgrind sample code uses locale after it's being freed :)
Thanks, Piotr
-
Ken Thomases -
Marvin -
Piotr Caban -
Piotr Caban