From: yaoyongjie <yaoyongjie@uniontech.com> Kingsoft Typing Pass failed to start probabilistically due to a deadlock between two threads registering and logging out of the same class. The replication scenario is as follows: |A thread (registration class) | B thread (deregistration class)| |-------------- |---------------| | NtUserRegisterClassExWOW | NtUserUnregisterClass | | create_class | -- | | -- | destroy_class | | find_shared_session_object | -- | | find failed, destroy_class | -- | | free class | -- | | -- | user_lock | | -- | remove class from list | | -- | throw exception(class has freeed by thread A) | Thread B threw an exception and will not call user_unlock. The upper layer application handled the memory access exception thrown by thread B, but due to If user_unlock is not called, the lock will remain occupied, and other threads will continue to wait when calling user_rock. Change-Id: I7352f3a7cdfb4699628e85d976f5f87dc11f4325 --- dlls/win32u/class.c | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/dlls/win32u/class.c b/dlls/win32u/class.c index 5478088a4f4..1daa3f19810 100644 --- a/dlls/win32u/class.c +++ b/dlls/win32u/class.c @@ -547,6 +547,14 @@ ATOM WINAPI NtUserRegisterClassExWOW( const WNDCLASSEXW *wc, UNICODE_STRING *nam class->local = !is_builtin && !(wc->style & CS_GLOBALCLASS); + /* Other non-null values must be set by caller */ + if (wc->hIcon && !wc->hIconSm) + sm_icon = CopyImage( wc->hIcon, IMAGE_ICON, + get_system_metrics( SM_CXSMICON ), + get_system_metrics( SM_CYSMICON ), + LR_COPYFROMRESOURCE ); + + user_lock(); SERVER_START_REQ( create_class ) { req->local = class->local; @@ -564,32 +572,25 @@ ATOM WINAPI NtUserRegisterClassExWOW( const WNDCLASSEXW *wc, UNICODE_STRING *nam SERVER_END_REQ; if (!ret) { + NtUserDestroyCursor( sm_icon, 0 ); free( class ); + user_unlock(); return 0; } if (!(shared = find_shared_session_object( locator.id, locator.offset ))) { ERR( "Failed to get shared session object for window class\n" ); - SERVER_START_REQ( destroy_class ) - { - req->instance = wine_server_client_ptr( instance ); - wine_server_add_data( req, name->Buffer, name->Length ); - wine_server_call( req ); - } - SERVER_END_REQ; - free( class ); + + /** + * If we fail to get the shared object, the class must be destroyed and freed by other thread, + * so we must not free it here. + */ + NtUserDestroyCursor( sm_icon, 0 ); + user_unlock(); return 0; } - /* Other non-null values must be set by caller */ - if (wc->hIcon && !wc->hIconSm) - sm_icon = CopyImage( wc->hIcon, IMAGE_ICON, - get_system_metrics( SM_CXSMICON ), - get_system_metrics( SM_CYSMICON ), - LR_COPYFROMRESOURCE ); - - user_lock(); if (class->local) list_add_head( &class_list, &class->entry ); else list_add_tail( &class_list, &class->entry ); @@ -621,6 +622,8 @@ BOOL WINAPI NtUserUnregisterClass( UNICODE_STRING *name, HINSTANCE instance, /* create the desktop window to trigger builtin class registration */ get_desktop_window(); + user_lock(); + SERVER_START_REQ( destroy_class ) { req->instance = wine_server_client_ptr( instance ); @@ -628,11 +631,14 @@ BOOL WINAPI NtUserUnregisterClass( UNICODE_STRING *name, HINSTANCE instance, if (!wine_server_call_err( req )) class = wine_server_get_ptr( reply->client_ptr ); } SERVER_END_REQ; - if (!class) return FALSE; + if (!class) + { + user_unlock(); + return FALSE; + } TRACE( "%p\n", class ); - user_lock(); if (class->dce) free_dce( class->dce, 0, &drawables ); list_remove( &class->entry ); if (class->hbrBackground > (HBRUSH)(COLOR_GRADIENTINACTIVECAPTION + 1)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10271