[PATCH v2 0/1] MR1456: imm32: Prevent deadlock on unload
We need to make sure all (important) cleanup is finished when we exit DllMain, otherwise we might already unload krnl386 and deadlock Since we can't have a synchronous DestroyWindow, use an extra message Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52511 Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> Code flow is like follows: - DOSVM_Exit -> RtlExitUserThread -> LdrShutdownThread - acquire loader lock - send DLL_THREAD_DETACH to imm32 - - calls DestroyWindow on its window - - WM_DESTROY is handled asynchonously! - send DLL_THREAD_DETACH to krnl386.exe - - TASK_ExitTask - - try get win16 lock, is blocked by ime32 meanwhile, at ime32 - handles WM_DESTROY - already have win16 lock from user32 - __wine_ime_wnd_proc (WM_DESTROY) -> imm_couninit_thread -> CoRevokeInitializeSpy - LdrGetProcedureAddress (want CoRevokeInitializeSpy) - try get loader_lock, is blocked Solution is to wait in ime32 DllMain until the cleanup as happened, aka CoRevokeInitializeSpy has finished. -- v2: imm32: Prevent deadlock on unload https://gitlab.winehq.org/wine/wine/-/merge_requests/1456
From: Fabian Maurer <dark.shadow4(a)web.de> We need to make sure all (important) cleanup is finished when we exit DllMain, otherwise we might already unload krnl386 and deadlock Since we can't have a synchronous DestroyWindow, use an extra message Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52511 Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/imm32/imm.c | 21 ++++++++++++++------- dlls/user32/misc.c | 2 +- dlls/user32/user_main.c | 1 + dlls/user32/user_private.h | 2 ++ 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/dlls/imm32/imm.c b/dlls/imm32/imm.c index 780d544c0e9..3a334bc0de5 100644 --- a/dlls/imm32/imm.c +++ b/dlls/imm32/imm.c @@ -51,6 +51,7 @@ static UINT WM_MSIME_RECONVERTREQUEST; static UINT WM_MSIME_RECONVERT; static UINT WM_MSIME_QUERYPOSITION; static UINT WM_MSIME_DOCUMENTFEED; +static UINT WM_WINE_IME_DESTROY; typedef struct _tagImmHkl{ struct list entry; @@ -554,7 +555,10 @@ static void IMM_FreeAllImmHkl(void) FreeLibrary(ptr->hIME); } if (ptr->UIWnd) + { + SendMessageA(ptr->UIWnd, WM_WINE_IME_DESTROY, 0, 0); DestroyWindow(ptr->UIWnd); + } HeapFree(GetProcessHeap(),0,ptr); } } @@ -3185,6 +3189,7 @@ static void init_messages(void) WM_MSIME_RECONVERT = RegisterWindowMessageW(L"MSIMEReconvert"); WM_MSIME_QUERYPOSITION = RegisterWindowMessageW(L"MSIMEQueryPosition"); WM_MSIME_DOCUMENTFEED = RegisterWindowMessageW(L"MSIMEDocumentFeed"); + WM_WINE_IME_DESTROY = RegisterWindowMessageW(L"__WINE_IME_DESTROY"); initialized = TRUE; } @@ -3198,16 +3203,18 @@ LRESULT WINAPI __wine_ime_wnd_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lp init_messages(); return TRUE; - case WM_DESTROY: + case WM_IME_INTERNAL: + return ime_internal_msg(wparam, lparam); + } + + if (msg == WM_WINE_IME_DESTROY) + { + HWND default_hwnd = ImmGetDefaultIMEWnd(0); + if (!default_hwnd || hwnd == default_hwnd) { - HWND default_hwnd = ImmGetDefaultIMEWnd(0); - if (!default_hwnd || hwnd == default_hwnd) - imm_couninit_thread(TRUE); + imm_couninit_thread(TRUE); } return TRUE; - - case WM_IME_INTERNAL: - return ime_internal_msg(wparam, lparam); } if (is_ime_ui_msg(msg)) diff --git a/dlls/user32/misc.c b/dlls/user32/misc.c index 879e8b32a0f..3b822ec047f 100644 --- a/dlls/user32/misc.c +++ b/dlls/user32/misc.c @@ -38,7 +38,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(win); BOOL WINAPI ImmSetActiveContext(HWND, HIMC, BOOL); #define IMM_INIT_MAGIC 0x19650412 -static LRESULT (WINAPI *imm_ime_wnd_proc)( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam, BOOL ansi); +LRESULT (WINAPI *imm_ime_wnd_proc)( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam, BOOL ansi); /* USER signal proc flags and codes */ /* See UserSignalProc for comments */ diff --git a/dlls/user32/user_main.c b/dlls/user32/user_main.c index 0ddbd710b6b..4a429540541 100644 --- a/dlls/user32/user_main.c +++ b/dlls/user32/user_main.c @@ -276,6 +276,7 @@ BOOL WINAPI DllMain( HINSTANCE inst, DWORD reason, LPVOID reserved ) thread_detach(); break; case DLL_PROCESS_DETACH: + imm_ime_wnd_proc = 0; /* Make sure we don't call into imm32 anymore */ FreeLibrary(imm32_module); break; } diff --git a/dlls/user32/user_private.h b/dlls/user32/user_private.h index 430ac826f7e..e22d4fecc24 100644 --- a/dlls/user32/user_private.h +++ b/dlls/user32/user_private.h @@ -184,4 +184,6 @@ static inline void mirror_rect( const RECT *window_rect, RECT *rect ) rect->right = width - tmp; } +extern LRESULT (WINAPI *imm_ime_wnd_proc)( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam, BOOL ansi); + #endif /* __WINE_USER_PRIVATE_H */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1456
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=126511 Your paranoid android. === debian11 (32 bit report) === imm32: imm32.c:2309: Test failed: CoGetApartmentType returned 0 imm32.c:2309: Test failed: type 3 imm32.c:2331: Test failed: CoGetApartmentType returned 0 imm32.c:2331: Test failed: type 3 imm32.c:2341: Test failed: CoGetApartmentType returned 0 imm32.c:2341: Test failed: type 1 imm32.c:2345: Test failed: CoGetApartmentType returned 0 imm32.c:2345: Test failed: type 1 imm32.c:2347: Test failed: type 1 imm32.c:2349: Test failed: CoGetApartmentType returned 0 imm32.c:2349: Test failed: type 1 === debian11 (32 bit zh:CN report) === imm32: imm32.c:2309: Test failed: CoGetApartmentType returned 0 imm32.c:2309: Test failed: type 3 imm32.c:2331: Test failed: CoGetApartmentType returned 0 imm32.c:2331: Test failed: type 3 imm32.c:2341: Test failed: CoGetApartmentType returned 0 imm32.c:2341: Test failed: type 1 imm32.c:2345: Test failed: CoGetApartmentType returned 0 imm32.c:2345: Test failed: type 1 imm32.c:2347: Test failed: type 1 imm32.c:2349: Test failed: CoGetApartmentType returned 0 imm32.c:2349: Test failed: type 1 === debian11b (64 bit WoW report) === imm32: imm32.c:2309: Test failed: CoGetApartmentType returned 0 imm32.c:2309: Test failed: type 3 imm32.c:2331: Test failed: CoGetApartmentType returned 0 imm32.c:2331: Test failed: type 3 imm32.c:2341: Test failed: CoGetApartmentType returned 0 imm32.c:2341: Test failed: type 1 imm32.c:2345: Test failed: CoGetApartmentType returned 0 imm32.c:2345: Test failed: type 1 imm32.c:2347: Test failed: type 1 imm32.c:2349: Test failed: CoGetApartmentType returned 0 imm32.c:2349: Test failed: type 1
participants (3)
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Marvin