[PATCH] winex11.drv: Protect XOpenIM with a critical section.

11 Sep 2019

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

I could reproduce for "Far Manager v2.0 build 1807 x86"
that "double free or corruption (!prev)" bug.

After some debugging and additional logging I guess this is a race
between two threads living in Far.exe.
Both run through XOpenIM at the same time, the main process by calling
ToUnicodeEx, the other thread by CreateWindowExW.
Therefore sometimes freeing the same memory. Sometimes just freezing.

Because of this being timing related, I cannot say if this is really
a regression.

A comment above open_xim reads:
"Should always be called with the x11 lock held"
Got this "x11 lock" removed in 2012?

This patch adds a critical section for all the contents of open_xim,
because I am unsure if just protecting XOpenIM would be sufficient.


I got the backtraces below, in Debian 10, current wine git with some
logging. Library libx11-6 got also some logging.
Therefore the line information might be off a few lines:

Thread 1 hit Breakpoint 1, destroy (lcd=0x7c5a4d20) at ../../../src/xlibi18n/lcPublic.c:275
warning: Source file is more recent than executable.
275         fprintf(stderr, "%d: destroy pub=%p lcd=%p\n", mygettid(), pub, lcd); fflush(stderr);
Wine-gdb> bt
#0  destroy (lcd=0x7c5a4d20) at ../../../src/xlibi18n/lcPublic.c:275
#1  0xf79b5269 in _XCloseLC (lcd=0x7c5a4d20) at ../../../src/xlibi18n/lcWrap.c:338
#2  0xf79b52bf in _XlcCurrentLC () at ../../../src/xlibi18n/lcWrap.c:365
#3  0xf79ae2d5 in _Xlcmbstowcs (lcd=0x0, wstr=0x32cd48 L"À", str=0x7c60a7a2 "À", len=127) at ../../../src/xlibi18n/lcStd.c:132
#4  0xf79ae3a2 in _Xmbstowcs (wstr=0x32cd48 L"À", str=0x7c60a7a2 "À", len=127) at ../../../src/xlibi18n/lcStd.c:232
#5  0xf79c2e74 in parseline (depth=<optimized out>, tokenbuf=<optimized out>, im=<optimized out>, fp=<optimized out>) at ../../../../modules/im/ximcp/imLcPrs.c:626
#6  parsestringfile (fp=fp(a)entry=0x7c594240, im=im(a)entry=0x7c5fbe40, depth=depth(a)entry=0) at ../../../../modules/im/ximcp/imLcPrs.c:737
#7  0xf79c360f in _XimParseStringFile (fp=0x7c594240, im=0x7c5fbe40) at ../../../../modules/im/ximcp/imLcPrs.c:716
#8  0xf79c0f1e in _XimCreateDefaultTree (im=0x7c5fbe40) at ../../../../modules/im/ximcp/imLcIm.c:610
#9  _XimLocalOpenIM (im=0x7c5fbe40) at ../../../../modules/im/ximcp/imLcIm.c:700
#10 0xf79bf482 in _XimOpenIM (lcd=0x7c5a4d20, dpy=0x7c5facc0, rdb=0x0, res_name=0x0, res_class=0x0) at ../../../../modules/im/ximcp/imInt.c:229
#11 0xf79a5ba6 in XOpenIM (display=0x7c5facc0, rdb=0x0, res_name=0x0, res_class=0x0) at ../../../src/xlibi18n/IMWrap.c:121
#12 0xf7b42495 in open_xim (display=0x7c5facc0) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/xim.c:354
#13 0xf7b42deb in X11DRV_SetupXIM () at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/xim.c:474
#14 0xf7b3ee99 in x11drv_init_thread_data () at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/x11drv_main.c:682
#15 0xf7b15bb5 in thread_init_display () at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/x11drv.h:369
#16 0xf7b199a4 in X11DRV_ToUnicodeEx (virtKey=0, scanCode=0, lpKeyState=0x32f4e4 "", bufW=0x32f65c, bufW_size=1, flags=0, hkl=0x4070407) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/keyboard.c:2447
#17 0x7e89cc85 in ToUnicodeEx (virtKey=0, scanCode=0, lpKeyState=0x32f4e4 "", lpwStr=0x32f65c, size=1, flags=0, hkl=0x4070407) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/user32/input.c:868
#18 0x00486311 in ?? ()
#19 0x0049b690 in ?? ()
#20 0x004fd424 in ?? ()
#21 0x7b4744ba in call_process_entry () at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/kernel32/process.c:1226
#22 0x7b474600 in start_process (entry=0x4fd47a, peb=0x7ffdf000) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/kernel32/process.c:1293
#23 0x7b4744c6 in start_process_wrapper () at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/kernel32/process.c:1226
#24 0x00000000 in ?? ()


Thread 2 received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 63]
0xf79adc3a in destroy (lcd=0x7c5a4d20) at ../../../src/xlibi18n/lcPublic.c:275
275         fprintf(stderr, "%d: destroy pub=%p lcd=%p\n", mygettid(), pub, lcd); fflush(stderr);
Wine-gdb> bt
#0  0xf79adc3a in destroy (lcd=0x7c5a4d20) at ../../../src/xlibi18n/lcPublic.c:275
#1  0xf79b5269 in _XCloseLC (lcd=0x7c5a4d20) at ../../../src/xlibi18n/lcWrap.c:338
#2  0xf79b52bf in _XlcCurrentLC () at ../../../src/xlibi18n/lcWrap.c:365
#3  0xf79ae715 in _Xlcmbstoutf8 (lcd=0x0, ustr=0x8bd69c "Ǫ", str=0xf7711964 "ǫ", len=255) at ../../../src/xlibi18n/lcStd.c:324
#4  0xf79ae752 in _Xmbstoutf8 (ustr=0x8bd69c "Ǫ", str=0xf7711964 "ǫ", len=255) at ../../../src/xlibi18n/lcStd.c:357
#5  0xf79c3159 in parseline (depth=<optimized out>, tokenbuf=<optimized out>, im=<optimized out>, fp=<optimized out>) at ../../../../modules/im/ximcp/imLcPrs.c:642
#6  parsestringfile (fp=fp(a)entry=0xf7701030, im=im(a)entry=0xf77019d0, depth=depth(a)entry=0) at ../../../../modules/im/ximcp/imLcPrs.c:737
#7  0xf79c360f in _XimParseStringFile (fp=0xf7701030, im=0xf77019d0) at ../../../../modules/im/ximcp/imLcPrs.c:716
#8  0xf79c0f1e in _XimCreateDefaultTree (im=0xf77019d0) at ../../../../modules/im/ximcp/imLcIm.c:610
#9  _XimLocalOpenIM (im=0xf77019d0) at ../../../../modules/im/ximcp/imLcIm.c:700
#10 0xf79bf482 in _XimOpenIM (lcd=0x7c5a4d20, dpy=0xf77005d0, rdb=0x0, res_name=0x0, res_class=0x0) at ../../../../modules/im/ximcp/imInt.c:229
#11 0xf79a5ba6 in XOpenIM (display=0xf77005d0, rdb=0x0, res_name=0x0, res_class=0x0) at ../../../src/xlibi18n/IMWrap.c:121
#12 0xf7b42495 in open_xim (display=0xf77005d0) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/xim.c:354
#13 0xf7b42deb in X11DRV_SetupXIM () at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/xim.c:474
#14 0xf7b3ee99 in x11drv_init_thread_data () at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/x11drv_main.c:682
#15 0xf7b32a77 in thread_init_display () at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/x11drv.h:369
#16 0xf7b37350 in X11DRV_create_win_data (hwnd=0x3006e, window_rect=0x8bfd88, client_rect=0x8bfd88) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/window.c:1883
#17 0xf7b380d3 in X11DRV_WindowPosChanging (hwnd=0x3006e, insert_after=0x0, swp_flags=20, window_rect=0x8bfd88, client_rect=0x8bfd88, visible_rect=0x8bfbf0, surface=0x8bfb8c) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/winex11.drv/window.c:2252
#18 0x7e8fbf85 in set_window_pos (hwnd=0x3006e, insert_after=0x0, swp_flags=20, window_rect=0x8bfd88, client_rect=0x8bfd88, valid_rects=0x0) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/user32/winpos.c:2080
#19 0x7e8f0c38 in WIN_CreateWindowEx (cs=0x8bfe70, className=0x51b598, module=0x0, unicode=1) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/user32/win.c:1614
#20 0x7e8f13b8 in CreateWindowExW (exStyle=0, className=0x51b598, windowName=0x0, style=0, x=-2147483648, y=-2147483648, width=-2147483648, height=-2147483648, parent=0x0, menu=0x0, instance=0x0, data=0x0) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/user32/win.c:1798
#21 0x004f6387 in ?? ()
#22 0x7bca9dd4 in call_thread_func_wrapper () at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/ntdll/signal_i386.c:2628
#23 0x7bca9e37 in call_thread_func (entry=0x4f6310, arg=0x5776b8) at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/ntdll/signal_i386.c:2723
#24 0x7bca9dc6 in call_thread_entry () at /home/bernhard/data/entwicklung/2019/wine/wine-git/wine-git/dlls/ntdll/signal_i386.c:2628
#25 0x00000000 in ?? ()
---
 dlls/winex11.drv/xim.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/dlls/winex11.drv/xim.c b/dlls/winex11.drv/xim.c
index 44fe62e900..fdbb29151d 100644
--- a/dlls/winex11.drv/xim.c
+++ b/dlls/winex11.drv/xim.c
@@ -59,6 +59,15 @@ static XIMStyle ximStyle = 0;
 static XIMStyle ximStyleRoot = 0;
 static XIMStyle ximStyleRequest = STYLE_CALLBACK;
 
+static CRITICAL_SECTION xim_section;
+static CRITICAL_SECTION_DEBUG critsect_debug =
+{
+    0, 0, &xim_section,
+    { &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList },
+      0, 0, { (DWORD_PTR)(__FILE__ ": xim_section") }
+};
+static CRITICAL_SECTION xim_section = { &critsect_debug, -1, 0, 0, 0, 0 };
+
 static void X11DRV_ImmSetInternalString(DWORD dwOffset,
                                         DWORD selLength, LPWSTR lpComp, DWORD dwCompLen)
 {
@@ -334,17 +343,25 @@ static void X11DRV_DestroyIM(XIM xim, XPointer p, XPointer data)
  */
 static BOOL open_xim( Display *display )
 {
-    struct x11drv_thread_data *thread_data = x11drv_thread_data();
+    struct x11drv_thread_data *thread_data;
     XIMStyle ximStyleNone;
     XIMStyles *ximStyles = NULL;
     INT i;
     XIM xim;
     XIMCallback destroy;
+    TRACE("(%p)\n", display);
+
+    EnterCriticalSection( &xim_section );
+    TRACE("EnterCriticalSection( &xim_section ) after\n");
+
+    thread_data = x11drv_thread_data();
 
     xim = XOpenIM(display, NULL, NULL, NULL);
     if (xim == NULL)
     {
         WARN("Could not open input method.\n");
+        TRACE("LeaveCriticalSection( &xim_section ) before\n");
+        LeaveCriticalSection( &xim_section );
         return FALSE;
     }
 
@@ -364,6 +381,8 @@ static BOOL open_xim( Display *display )
     {
         WARN("Could not find supported input style.\n");
         XCloseIM(xim);
+        TRACE("LeaveCriticalSection( &xim_section ) before\n");
+        LeaveCriticalSection( &xim_section );
         return FALSE;
     }
     else
@@ -433,6 +452,10 @@ static BOOL open_xim( Display *display )
         thread_data->font_set = NULL;
 
     IME_UpdateAssociation(NULL);
+
+    TRACE("LeaveCriticalSection( &xim_section ) before\n");
+    LeaveCriticalSection( &xim_section );
+
     return TRUE;
 }
 
-- 
2.20.1

    

Bernhard Übelacker

Dmitry Timoshkov

Bernhard Übelacker

Jacek Caban

tags

participants (3)