Re: [PATCH v2 0/2] MR10271: Fixed thread safety issues with NtUserRegistClassExWOW and NtUserUnregisterClass.
Rémi Bernon (@rbernon) commented about dlls/win32u/class.c:
- 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; I don't think that's true? With the new user_lock scope, class creation and insertion in the process linked list happens atomically. Symmetrically, class destruction and removal from the process class list also happens atomically. So only scenario that `find_shared_session_object` can fail, is some general critical error.
I don't think we care that much about what happens in that case, but it seems to me that class pointer is exclusively owned by the calling thread in all case until the class is inserted in the process list and user critical section exited, and the pointer can and should always be freed on error. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10271#note_133371
participants (1)
-
Rémi Bernon