[PATCH 0/4] MR7512: win32u: Introduce shared memory objects for windows.
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/protocol.def | 1 - server/window.c | 1 - 2 files changed, 2 deletions(-) diff --git a/server/protocol.def b/server/protocol.def index 4f712b4e4e6..cc658915da0 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2573,7 +2573,6 @@ enum message_type user_handle_t last_active; /* last active popup */ process_id_t pid; /* process owning the window */ thread_id_t tid; /* thread owning the window */ - atom_t atom; /* class atom */ int is_unicode; /* ANSI or unicode */ unsigned int dpi_context; /* window DPI context */ @END diff --git a/server/window.c b/server/window.c index f7f9d5e517f..afd4564e206 100644 --- a/server/window.c +++ b/server/window.c @@ -2331,7 +2331,6 @@ DECL_HANDLER(get_window_info) { reply->tid = get_thread_id( win->thread ); reply->pid = get_process_id( win->thread->process ); - reply->atom = win->class ? get_class_atom( win->class ) : DESKTOP_ATOM; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7512
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/win32u_private.h | 1 + dlls/win32u/window.c | 4 ++++ dlls/win32u/winstation.c | 38 +++++++++++++++++++++++++++++++++++- server/protocol.def | 25 ++++++++++++++++-------- server/window.c | 26 ++++++++++++++++++++++-- 5 files changed, 83 insertions(+), 11 deletions(-) diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index b4738ed6b16..3bdfa47b439 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -222,6 +222,7 @@ struct object_lock extern NTSTATUS get_shared_desktop( struct object_lock *lock, const desktop_shm_t **desktop_shm ); extern NTSTATUS get_shared_queue( struct object_lock *lock, const queue_shm_t **queue_shm ); extern NTSTATUS get_shared_input( UINT tid, struct object_lock *lock, const input_shm_t **input_shm ); +extern void set_shared_user_object( HANDLE handle, struct obj_locator locator ); extern BOOL is_virtual_desktop(void); diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 1cea6d25a4b..6dbfaebb497 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -5305,6 +5305,7 @@ static WND *create_window_handle( HWND parent, HWND owner, UNICODE_STRING *name, UINT dpi_context = get_thread_dpi_awareness_context(); HWND handle = 0, full_parent = 0, full_owner = 0; struct tagCLASS *class = NULL; + struct obj_locator locator; int extra_bytes = 0; WND *win; @@ -5320,6 +5321,7 @@ static WND *create_window_handle( HWND parent, HWND owner, UNICODE_STRING *name, wine_server_add_data( req, name->Buffer, name->Length ); if (!wine_server_call_err( req )) { + locator = reply->locator; handle = wine_server_ptr_handle( reply->handle ); full_parent = wine_server_ptr_handle( reply->parent ); full_owner = wine_server_ptr_handle( reply->owner ); @@ -5348,6 +5350,8 @@ static WND *create_window_handle( HWND parent, HWND owner, UNICODE_STRING *name, return NULL; } + set_shared_user_object( handle, locator ); + if (!parent) /* if parent is 0 we don't have a desktop window yet */ { struct ntuser_thread_info *thread_info = NtUserGetThreadInfo(); diff --git a/dlls/win32u/winstation.c b/dlls/win32u/winstation.c index b350b706454..5850f1121f0 100644 --- a/dlls/win32u/winstation.c +++ b/dlls/win32u/winstation.c @@ -52,6 +52,12 @@ struct shared_input_cache DWORD tid; }; +struct shared_user_object_cache +{ + const shared_object_t *object; + UINT64 id; +}; + struct session_thread_data { const shared_object_t *shared_desktop; /* thread desktop shared session cached object */ @@ -328,6 +334,26 @@ NTSTATUS get_shared_input( UINT tid, struct object_lock *lock, const input_shm_t return status; } +#define NB_USER_HANDLES ((LAST_USER_HANDLE - FIRST_USER_HANDLE + 1) >> 1) +#define USER_HANDLE_TO_INDEX(handle) ((LOWORD(handle) - FIRST_USER_HANDLE) >> 1) + +static struct shared_user_object_cache user_objects[NB_USER_HANDLES]; +static pthread_mutex_t user_objects_lock = PTHREAD_MUTEX_INITIALIZER; + +void set_shared_user_object( HANDLE handle, struct obj_locator locator ) +{ + WORD index = USER_HANDLE_TO_INDEX( handle ); + struct shared_user_object_cache *cache; + + if (index >= NB_USER_HANDLES) return; + cache = user_objects + index; + + pthread_mutex_lock( &user_objects_lock ); + cache->id = locator.id; + cache->object = find_shared_session_object( locator ); + pthread_mutex_unlock( &user_objects_lock ); +} + BOOL is_virtual_desktop(void) { struct object_lock lock = OBJECT_LOCK_INIT; @@ -777,6 +803,7 @@ HWND get_desktop_window(void) static const WCHAR wine_service_station_name[] = {'_','_','w','i','n','e','s','e','r','v','i','c','e','_','w','i','n','s','t','a','t','i','o','n',0}; struct ntuser_thread_info *thread_info = NtUserGetThreadInfo(); + struct obj_locator top_locator, msg_locator; WCHAR name[MAX_PATH]; BOOL is_service; @@ -796,6 +823,8 @@ HWND get_desktop_window(void) { thread_info->top_window = reply->top_window; thread_info->msg_window = reply->msg_window; + top_locator = reply->top_locator; + msg_locator = reply->msg_locator; } } SERVER_END_REQ; @@ -878,13 +907,20 @@ HWND get_desktop_window(void) { thread_info->top_window = reply->top_window; thread_info->msg_window = reply->msg_window; + top_locator = reply->top_locator; + msg_locator = reply->msg_locator; } } SERVER_END_REQ; } if (!thread_info->top_window) ERR_(win)( "failed to create desktop window\n" ); - else user_driver->pSetDesktopWindow( UlongToHandle( thread_info->top_window )); + else + { + set_shared_user_object( UlongToHandle( thread_info->top_window ), top_locator ); + set_shared_user_object( UlongToHandle( thread_info->msg_window ), msg_locator ); + user_driver->pSetDesktopWindow( UlongToHandle( thread_info->top_window )); + } register_builtin_classes(); return UlongToHandle( thread_info->top_window ); diff --git a/server/protocol.def b/server/protocol.def index cc658915da0..44e952ecc91 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1007,11 +1007,17 @@ typedef volatile struct int keystate_lock; /* keystate is locked */ } input_shm_t; +typedef volatile struct +{ + int placeholder; +} window_shm_t; + typedef volatile union { desktop_shm_t desktop; queue_shm_t queue; input_shm_t input; + window_shm_t window; } object_shm_t; typedef volatile struct @@ -2531,12 +2537,13 @@ enum message_type unsigned int ex_style; /* window extended style */ VARARG(class,unicode_str); /* class name */ @REPLY - user_handle_t handle; /* created window */ - user_handle_t parent; /* full handle of parent */ - user_handle_t owner; /* full handle of owner */ - int extra; /* number of extra bytes */ - client_ptr_t class_ptr; /* pointer to class in client address space */ - unsigned int dpi_context; /* window DPI context */ + struct obj_locator locator; /* locator for the shared session object */ + user_handle_t handle; /* created window */ + user_handle_t parent; /* full handle of parent */ + user_handle_t owner; /* full handle of owner */ + int extra; /* number of extra bytes */ + client_ptr_t class_ptr; /* pointer to class in client address space */ + unsigned int dpi_context; /* window DPI context */ @END @@ -2550,8 +2557,10 @@ enum message_type @REQ(get_desktop_window) int force; /* force creation if it doesn't exist */ @REPLY - user_handle_t top_window; /* handle to the desktop window */ - user_handle_t msg_window; /* handle to the top-level HWND_MESSAGE parent */ + user_handle_t top_window; /* handle to the desktop window */ + user_handle_t msg_window; /* handle to the top-level HWND_MESSAGE parent */ + struct obj_locator top_locator; /* locator for the desktop window shared session object */ + struct obj_locator msg_locator; /* locator for the message window shared session object */ @END diff --git a/server/window.c b/server/window.c index afd4564e206..1fe681b5d56 100644 --- a/server/window.c +++ b/server/window.c @@ -29,6 +29,7 @@ #include "winbase.h" #include "ntuser.h" +#include "file.h" #include "object.h" #include "request.h" #include "thread.h" @@ -94,6 +95,7 @@ struct window struct property *properties; /* window properties array */ int nb_extra_bytes; /* number of extra bytes */ char *extra_bytes; /* extra bytes storage */ + const window_shm_t *shared; /* window in session shared memory */ }; static void window_dump( struct object *obj, int verbose ); @@ -180,6 +182,8 @@ static void window_destroy( struct object *obj ) memset( win->extra_bytes, 0x55, win->nb_extra_bytes ); free( win->extra_bytes ); } + + if (win->shared) free_shared_object( win->shared ); } /* retrieve a pointer to a window from its handle */ @@ -662,10 +666,18 @@ static struct window *create_window( struct window *parent, struct window *owner win->properties = NULL; win->nb_extra_bytes = 0; win->extra_bytes = NULL; + win->shared = NULL; win->window_rect = win->visible_rect = win->surface_rect = win->client_rect = empty_rect; list_init( &win->children ); list_init( &win->unlinked ); + if (!(win->shared = alloc_shared_object())) goto failed; + SHARED_WRITE_BEGIN( win->shared, window_shm_t ) + { + shared->placeholder = 0; + } + SHARED_WRITE_END; + if (extra_bytes) { if (!(win->extra_bytes = mem_alloc( extra_bytes ))) goto failed; @@ -2204,6 +2216,7 @@ DECL_HANDLER(create_window) win->style = req->style; win->ex_style = req->ex_style; + reply->locator = get_shared_object_locator( win->shared ); reply->handle = win->handle; reply->parent = win->parent ? win->parent->handle : 0; reply->owner = win->owner; @@ -2255,6 +2268,7 @@ DECL_HANDLER(destroy_window) DECL_HANDLER(get_desktop_window) { struct desktop *desktop = get_thread_desktop( current, 0 ); + struct window *win; if (!desktop) return; @@ -2279,8 +2293,16 @@ DECL_HANDLER(get_desktop_window) } } - reply->top_window = desktop->top_window ? desktop->top_window->handle : 0; - reply->msg_window = desktop->msg_window ? desktop->msg_window->handle : 0; + if ((win = desktop->top_window)) + { + reply->top_window = win->handle; + reply->top_locator = get_shared_object_locator( win->shared ); + } + if ((win = desktop->msg_window)) + { + reply->msg_window = win->handle; + reply->msg_locator = get_shared_object_locator( win->shared ); + } release_object( desktop ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7512
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/protocol.def | 2 +- server/window.c | 124 ++++++++++++++++++++++---------------------- 2 files changed, 64 insertions(+), 62 deletions(-) diff --git a/server/protocol.def b/server/protocol.def index 44e952ecc91..2d6c712d8dc 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1009,7 +1009,7 @@ typedef volatile struct typedef volatile struct { - int placeholder; + user_handle_t handle; /* full handle for this window */ } window_shm_t; typedef volatile union diff --git a/server/window.c b/server/window.c index 1fe681b5d56..9357536aebf 100644 --- a/server/window.c +++ b/server/window.c @@ -61,7 +61,6 @@ struct window struct list children; /* list of children in Z-order */ struct list unlinked; /* list of children not linked in the Z-order list */ struct list entry; /* entry in parent's children list */ - user_handle_t handle; /* full handle for this window */ struct thread *thread; /* thread owning the window */ struct desktop *desktop; /* desktop that the window belongs to */ struct window_class *class; /* window class */ @@ -157,14 +156,14 @@ static void window_dump( struct object *obj, int verbose ) { struct window *win = (struct window *)obj; assert( obj->ops == &window_ops ); - fprintf( stderr, "window %p handle %x\n", win, win->handle ); + fprintf( stderr, "window %p handle %x\n", win, win->shared->handle ); } static void window_destroy( struct object *obj ) { struct window *win = (struct window *)obj; - assert( !win->handle ); + assert( !win->shared->handle ); if (win->parent) { @@ -375,7 +374,7 @@ static int link_window( struct window *win, struct window *previous ) { struct window *next = LIST_ENTRY( entry, struct window, entry ); if (!(next->ex_style & WS_EX_TOPMOST)) break; - if (next->handle == win->owner) /* keep it above owner */ + if (next->shared->handle == win->owner) /* keep it above owner */ { win->ex_style |= WS_EX_TOPMOST; break; @@ -565,7 +564,7 @@ static void detach_window_thread( struct window *win ) { if (win->update_region) inc_queue_paint_count( thread, -1 ); if (win->paint_flags & PAINT_INTERNAL) inc_queue_paint_count( thread, -1 ); - queue_cleanup_window( thread, win->handle ); + queue_cleanup_window( thread, win->shared->handle ); } assert( thread->desktop_users > 0 ); thread->desktop_users--; @@ -597,7 +596,7 @@ void post_desktop_message( struct desktop *desktop, unsigned int message, lparam_t wparam, lparam_t lparam ) { struct window *win = desktop->top_window; - if (win && win->thread) post_message( win->handle, message, wparam, lparam ); + if (win && win->thread) post_message( win->shared->handle, message, wparam, lparam ); } /* create a new window structure (note: the window is not linked in the window tree) */ @@ -608,6 +607,7 @@ static struct window *create_window( struct window *parent, struct window *owner struct window *win = NULL; struct desktop *desktop; struct window_class *class; + user_handle_t handle = 0; if (!(desktop = get_thread_desktop( current, DESKTOP_CREATEWINDOW ))) return NULL; @@ -640,7 +640,7 @@ static struct window *create_window( struct window *parent, struct window *owner if (!(win = alloc_object( &window_ops ))) goto failed; win->parent = parent ? (struct window *)grab_object( parent ) : NULL; - win->owner = owner ? owner->handle : 0; + win->owner = owner ? owner->shared->handle : 0; win->thread = current; win->desktop = desktop; win->class = class; @@ -672,9 +672,12 @@ static struct window *create_window( struct window *parent, struct window *owner list_init( &win->unlinked ); if (!(win->shared = alloc_shared_object())) goto failed; + if (!(handle = alloc_user_handle( win, USER_WINDOW ))) goto failed; + win->last_active = win->shared->handle; + SHARED_WRITE_BEGIN( win->shared, window_shm_t ) { - shared->placeholder = 0; + shared->handle = handle; } SHARED_WRITE_END; @@ -684,8 +687,6 @@ static struct window *create_window( struct window *parent, struct window *owner memset( win->extra_bytes, 0, extra_bytes ); win->nb_extra_bytes = extra_bytes; } - if (!(win->handle = alloc_user_handle( win, USER_WINDOW ))) goto failed; - win->last_active = win->handle; /* if parent belongs to a different thread and the window isn't */ /* top-level, attach the two threads */ @@ -720,15 +721,8 @@ static struct window *create_window( struct window *parent, struct window *owner return win; failed: - if (win) - { - if (win->handle) - { - free_user_handle( win->handle ); - win->handle = 0; - } - release_object( win ); - } + if (win) release_object( win ); + if (handle) free_user_handle( handle ); release_object( desktop ); release_class( class ); return NULL; @@ -793,7 +787,7 @@ int make_window_active( user_handle_t window ) owner = win; while (owner) { - owner->last_active = win->handle; + owner->last_active = win->shared->handle; owner = get_user_object( owner->owner, USER_WINDOW ); } return 1; @@ -922,7 +916,7 @@ static void append_window_to_list( struct window *win, struct thread *thread, at { if (thread && win->thread != thread) return; if (atom && get_class_atom( win->class ) != atom) return; - if (*count < max_count) handles[*count] = win->handle; + if (*count < max_count) handles[*count] = win->shared->handle; (*count)++; } @@ -1011,7 +1005,7 @@ static int get_window_children_from_point( struct window *parent, int x, int y, } /* now add window to the array */ - if (!add_handle_to_array( array, ptr->handle )) return 0; + if (!add_handle_to_array( array, ptr->shared->handle )) return 0; } return 1; } @@ -1030,9 +1024,9 @@ user_handle_t shallow_window_from_point( struct desktop *desktop, int x, int y ) int x_child = x, y_child = y; if (!is_point_in_window( ptr, &x_child, &y_child, 0 )) continue; /* skip it */ - return ptr->handle; + return ptr->shared->handle; } - return desktop->top_window->handle; + return desktop->top_window->shared->handle; } /* return thread of top-most window containing point (in absolute raw coords) */ @@ -1072,7 +1066,7 @@ static int all_windows_from_point( struct window *top, int x, int y, unsigned in if (!get_window_children_from_point( top, x, y, array )) return 0; } /* now add window to the array */ - if (!add_handle_to_array( array, top->handle )) return 0; + if (!add_handle_to_array( array, top->shared->handle )) return 0; return 1; } @@ -1137,13 +1131,13 @@ user_handle_t find_window_to_repaint( user_handle_t parent, struct thread *threa { /* check that it is a child of the specified parent */ for (ptr = win; ptr; ptr = ptr->parent) - if (ptr->handle == parent) break; + if (ptr->shared->handle == parent) break; /* otherwise don't return any window, we don't repaint a child before its parent */ if (!ptr) win = NULL; } if (!win) return 0; win->paint_flags &= ~PAINT_INTERNAL; - return win->handle; + return win->shared->handle; } @@ -2106,8 +2100,9 @@ static void set_window_region( struct window *win, struct region *region, int re void free_window_handle( struct window *win ) { struct window *child, *next; + user_handle_t handle; - assert( win->handle ); + assert( win->shared->handle ); /* hide the window */ if (is_visible(win)) @@ -2127,19 +2122,19 @@ void free_window_handle( struct window *win ) /* destroy all children */ LIST_FOR_EACH_ENTRY_SAFE( child, next, &win->children, struct window, entry ) { - if (!child->handle) continue; + if (!child->shared->handle) continue; if (!win->thread || !child->thread || win->thread == child->thread) free_window_handle( child ); else - send_notify_message( child->handle, WM_WINE_DESTROYWINDOW, 0, 0 ); + send_notify_message( child->shared->handle, WM_WINE_DESTROYWINDOW, 0, 0 ); } LIST_FOR_EACH_ENTRY_SAFE( child, next, &win->children, struct window, entry ) { - if (!child->handle) continue; + if (!child->shared->handle) continue; if (!win->thread || !child->thread || win->thread == child->thread) free_window_handle( child ); else - send_notify_message( child->handle, WM_WINE_DESTROYWINDOW, 0, 0 ); + send_notify_message( child->shared->handle, WM_WINE_DESTROYWINDOW, 0, 0 ); } /* reset global window pointers, if the corresponding window is destroyed */ @@ -2147,8 +2142,8 @@ void free_window_handle( struct window *win ) if (win == win->desktop->shell_listview) win->desktop->shell_listview = NULL; if (win == win->desktop->progman_window) win->desktop->progman_window = NULL; if (win == win->desktop->taskman_window) win->desktop->taskman_window = NULL; - free_hotkeys( win->desktop, win->handle ); - cleanup_clipboard_window( win->desktop, win->handle ); + free_hotkeys( win->desktop, win->shared->handle ); + cleanup_clipboard_window( win->desktop, win->shared->handle ); destroy_properties( win ); if (is_desktop_window(win)) { @@ -2159,14 +2154,21 @@ void free_window_handle( struct window *win ) } else if (is_desktop_window( win->parent )) { - post_message( win->parent->handle, WM_PARENTNOTIFY, WM_DESTROY, win->handle ); + post_message( win->parent->shared->handle, WM_PARENTNOTIFY, WM_DESTROY, win->shared->handle ); } detach_window_thread( win ); if (win->parent) set_parent_window( win, NULL ); - free_user_handle( win->handle ); - win->handle = 0; + + SHARED_WRITE_BEGIN( win->shared, window_shm_t ) + { + handle = win->shared->handle; + shared->handle = 0; + } + SHARED_WRITE_END; + free_user_handle( handle ); + release_object( win ); } @@ -2217,8 +2219,8 @@ DECL_HANDLER(create_window) win->ex_style = req->ex_style; reply->locator = get_shared_object_locator( win->shared ); - reply->handle = win->handle; - reply->parent = win->parent ? win->parent->handle : 0; + reply->handle = win->shared->handle; + reply->parent = win->parent ? win->parent->shared->handle : 0; reply->owner = win->owner; reply->extra = win->nb_extra_bytes; reply->dpi_context = win->dpi_context; @@ -2239,8 +2241,8 @@ DECL_HANDLER(set_parent) set_error( STATUS_INVALID_PARAMETER ); return; } - reply->old_parent = win->parent->handle; - reply->full_parent = parent ? parent->handle : 0; + reply->old_parent = win->parent->shared->handle; + reply->full_parent = parent ? parent->shared->handle : 0; set_parent_window( win, parent ); reply->dpi_context = win->dpi_context; } @@ -2295,12 +2297,12 @@ DECL_HANDLER(get_desktop_window) if ((win = desktop->top_window)) { - reply->top_window = win->handle; + reply->top_window = win->shared->handle; reply->top_locator = get_shared_object_locator( win->shared ); } if ((win = desktop->msg_window)) { - reply->msg_window = win->handle; + reply->msg_window = win->shared->handle; reply->msg_locator = get_shared_object_locator( win->shared ); } release_object( desktop ); @@ -2332,7 +2334,7 @@ DECL_HANDLER(set_window_owner) } reply->prev_owner = win->owner; - reply->full_owner = win->owner = owner ? owner->handle : 0; + reply->full_owner = win->owner = owner ? owner->shared->handle : 0; } @@ -2343,8 +2345,8 @@ DECL_HANDLER(get_window_info) if (!win) return; - reply->full_handle = win->handle; - reply->last_active = win->handle; + reply->full_handle = win->shared->handle; + reply->last_active = win->shared->handle; reply->is_unicode = win->is_unicode; reply->dpi_context = win->dpi_context; @@ -2424,7 +2426,7 @@ DECL_HANDLER(get_window_parents) if (len && ((data = set_reply_data_size( len )))) { for (ptr = win->parent; ptr && len; ptr = ptr->parent, len -= sizeof(*data)) - *data++ = ptr->handle; + *data++ = ptr->shared->handle; } } @@ -2566,18 +2568,18 @@ DECL_HANDLER(get_window_tree) if (win->parent) { struct window *parent = win->parent; - reply->parent = parent->handle; + reply->parent = parent->shared->handle; reply->owner = win->owner; if (win->is_linked) { - if ((ptr = get_next_window( win ))) reply->next_sibling = ptr->handle; - if ((ptr = get_prev_window( win ))) reply->prev_sibling = ptr->handle; + if ((ptr = get_next_window( win ))) reply->next_sibling = ptr->shared->handle; + if ((ptr = get_prev_window( win ))) reply->prev_sibling = ptr->shared->handle; } - if ((ptr = get_first_child( parent ))) reply->first_sibling = ptr->handle; - if ((ptr = get_last_child( parent ))) reply->last_sibling = ptr->handle; + if ((ptr = get_first_child( parent ))) reply->first_sibling = ptr->shared->handle; + if ((ptr = get_last_child( parent ))) reply->last_sibling = ptr->shared->handle; } - if ((ptr = get_first_child( win ))) reply->first_child = ptr->handle; - if ((ptr = get_last_child( win ))) reply->last_child = ptr->handle; + if ((ptr = get_first_child( win ))) reply->first_child = ptr->shared->handle; + if ((ptr = get_last_child( win ))) reply->last_child = ptr->shared->handle; } @@ -2668,7 +2670,7 @@ DECL_HANDLER(set_window_pos) reply->new_ex_style = win->ex_style; top = get_top_clipping_window( win ); - if (is_visible( top ) && (top->paint_flags & PAINT_HAS_SURFACE)) reply->surface_win = top->handle; + if (is_visible( top ) && (top->paint_flags & PAINT_HAS_SURFACE)) reply->surface_win = top->shared->handle; } @@ -2793,7 +2795,7 @@ DECL_HANDLER(get_visible_region) data = get_region_data_and_free( region, get_reply_max_size(), &reply->total_size ); if (data) set_reply_data_ptr( data, reply->total_size ); } - reply->top_win = top->handle; + reply->top_win = top->shared->handle; reply->top_rect = top->surface_rect; if (!is_desktop_window(win)) @@ -2908,7 +2910,7 @@ DECL_HANDLER(get_update_region) } reply->flags = get_window_update_flags( win, from_child, flags, &win ); - reply->child = win->handle; + reply->child = win->shared->handle; if (flags & UPDATE_NOREGION) return; @@ -3127,10 +3129,10 @@ DECL_HANDLER(set_desktop_shell_windows) new_progman_window = desktop->progman_window; new_taskman_window = desktop->taskman_window; - reply->old_shell_window = new_shell_window ? new_shell_window->handle : 0; - reply->old_shell_listview = new_shell_listview ? new_shell_listview->handle : 0; - reply->old_progman_window = new_progman_window ? new_progman_window->handle : 0; - reply->old_taskman_window = new_taskman_window ? new_taskman_window->handle : 0; + reply->old_shell_window = new_shell_window ? new_shell_window->shared->handle : 0; + reply->old_shell_listview = new_shell_listview ? new_shell_listview->shared->handle : 0; + reply->old_progman_window = new_progman_window ? new_progman_window->shared->handle : 0; + reply->old_taskman_window = new_taskman_window ? new_taskman_window->shared->handle : 0; if (req->flags & SET_DESKTOP_SHELL_WINDOWS) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7512
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/win32u_private.h | 1 + dlls/win32u/window.c | 31 +++++------------ dlls/win32u/winstation.c | 64 ++++++++++++++++++++++++++++++++++++ server/hook.c | 2 +- server/protocol.def | 9 ++++- server/user.c | 18 ++++++++-- server/user.h | 2 +- server/window.c | 3 +- 8 files changed, 100 insertions(+), 30 deletions(-) diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index 3bdfa47b439..a85f0b2e292 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -222,6 +222,7 @@ struct object_lock extern NTSTATUS get_shared_desktop( struct object_lock *lock, const desktop_shm_t **desktop_shm ); extern NTSTATUS get_shared_queue( struct object_lock *lock, const queue_shm_t **queue_shm ); extern NTSTATUS get_shared_input( UINT tid, struct object_lock *lock, const input_shm_t **input_shm ); +extern NTSTATUS get_shared_window( HWND hwnd, struct object_lock *lock, const window_shm_t **window_shm ); extern void set_shared_user_object( HANDLE handle, struct obj_locator locator ); extern BOOL is_virtual_desktop(void); diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 6dbfaebb497..1eb8c5f40e9 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -182,36 +182,21 @@ HWND get_hwnd_message_parent(void) */ HWND get_full_window_handle( HWND hwnd ) { - WND *win; + struct object_lock lock = OBJECT_LOCK_INIT; + const window_shm_t *window_shm; + HWND handle = hwnd; + UINT status; if (!hwnd || (ULONG_PTR)hwnd >> 16) return hwnd; if (LOWORD(hwnd) <= 1 || LOWORD(hwnd) == 0xffff) return hwnd; /* do sign extension for -2 and -3 */ if (LOWORD(hwnd) >= (WORD)-3) return (HWND)(LONG_PTR)(INT16)LOWORD(hwnd); - if (!(win = get_win_ptr( hwnd ))) return hwnd; - - if (win == WND_DESKTOP) - { - if (LOWORD(hwnd) == LOWORD(get_desktop_window())) return get_desktop_window(); - else return get_hwnd_message_parent(); - } + while ((status = get_shared_window( hwnd, &lock, &window_shm )) == STATUS_PENDING) + handle = wine_server_ptr_handle( window_shm->handle ); + if (status) return hwnd; - if (win != WND_OTHER_PROCESS) - { - hwnd = win->obj.handle; - release_win_ptr( win ); - } - else /* may belong to another process */ - { - SERVER_START_REQ( get_window_info ) - { - req->handle = wine_server_user_handle( hwnd ); - if (!wine_server_call_err( req )) hwnd = wine_server_ptr_handle( reply->full_handle ); - } - SERVER_END_REQ; - } - return hwnd; + return handle; } /******************************************************************* diff --git a/dlls/win32u/winstation.c b/dlls/win32u/winstation.c index 5850f1121f0..8060e4fb469 100644 --- a/dlls/win32u/winstation.c +++ b/dlls/win32u/winstation.c @@ -354,6 +354,70 @@ void set_shared_user_object( HANDLE handle, struct obj_locator locator ) pthread_mutex_unlock( &user_objects_lock ); } +static NTSTATUS get_shared_user_object( HANDLE handle, unsigned int type, struct object_lock *lock, + const object_shm_t **object_shm ) +{ + WORD index = USER_HANDLE_TO_INDEX( handle ); + struct shared_user_object_cache *cache; + const shared_object_t *object; + UINT status = STATUS_SUCCESS; + BOOL valid = TRUE; + + if (index >= NB_USER_HANDLES) return STATUS_INVALID_PARAMETER; + cache = user_objects + index; + + pthread_mutex_lock( &user_objects_lock ); + if (!(object = cache->object)) + { + struct obj_locator locator; + + SERVER_START_REQ( get_shared_user_object ) + { + req->handle = wine_server_user_handle( handle ); + req->type = type; + wine_server_call( req ); + locator = reply->locator; + } + SERVER_END_REQ; + + cache->id = locator.id; + cache->object = find_shared_session_object( locator ); + if (!(object = cache->object)) status = STATUS_INVALID_HANDLE; + memset( lock, 0, sizeof(*lock) ); + } + + /* check object validity by comparing ids, within the object seqlock */ + if (!status) valid = cache->id == object->id; + pthread_mutex_unlock( &user_objects_lock ); + + if (!status && (!lock->id || !shared_object_release_seqlock( object, lock->seq ))) + { + shared_object_acquire_seqlock( object, &lock->seq ); + if (!(lock->id = object->id)) lock->id = -1; + *object_shm = &object->shm; + return STATUS_PENDING; + } + + if (!valid) + { + memset( cache, 0, sizeof(*cache) ); /* object has been invalidated, clear the cache */ + return STATUS_INVALID_HANDLE; + } + return status; +} + +NTSTATUS get_shared_window( HWND hwnd, struct object_lock *lock, const window_shm_t **window_shm ) +{ + const object_shm_t *object_shm; + UINT status = STATUS_SUCCESS; + + TRACE( "hwnd %p, lock %p, input_shm %p\n", hwnd, lock, window_shm ); + + status = get_shared_user_object( hwnd, NTUSER_OBJ_WINDOW, lock, &object_shm ); + if (status == STATUS_PENDING) *window_shm = &object_shm->window; + return status; +} + BOOL is_virtual_desktop(void) { struct object_lock lock = OBJECT_LOCK_INIT; diff --git a/server/hook.c b/server/hook.c index ffe7206369e..4583dc3f206 100644 --- a/server/hook.c +++ b/server/hook.c @@ -153,7 +153,7 @@ static struct hook *add_hook( struct desktop *desktop, struct process *process, } if (!(hook = mem_alloc( sizeof(*hook) ))) return NULL; - if (!(hook->handle = alloc_user_handle( hook, USER_HOOK ))) + if (!(hook->handle = alloc_user_handle( hook, USER_HOOK, NULL ))) { free( hook ); return NULL; diff --git a/server/protocol.def b/server/protocol.def index 2d6c712d8dc..bb4e66aedb2 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2578,7 +2578,6 @@ enum message_type @REQ(get_window_info) user_handle_t handle; /* handle to the window */ @REPLY - user_handle_t full_handle; /* full 32-bit handle */ user_handle_t last_active; /* last active popup */ process_id_t pid; /* process owning the window */ thread_id_t tid; /* thread owning the window */ @@ -2982,6 +2981,14 @@ enum coords_relative @END +(a)REQ(get_shared_user_object) + obj_handle_t handle; /* handle to the object */ + unsigned int type; /* expected type of the object */ +(a)REPLY + struct obj_locator locator; +(a)END + + /* Get/set information about a user object (window station or desktop) */ @REQ(set_user_object_info) obj_handle_t handle; /* handle to the object */ diff --git a/server/user.c b/server/user.c index 2d038a6ddbf..163db4f20af 100644 --- a/server/user.c +++ b/server/user.c @@ -20,11 +20,13 @@ #include "thread.h" #include "user.h" +#include "file.h" #include "request.h" struct user_handle { void *ptr; /* pointer to object */ + const volatile void *shm; /* object shared memory */ unsigned short type; /* object type (0 if free) */ unsigned short generation; /* generation counter */ }; @@ -84,17 +86,19 @@ static inline void *free_user_entry( struct user_handle *ptr ) void *ret; ret = ptr->ptr; ptr->ptr = freelist; + ptr->shm = NULL; ptr->type = 0; freelist = ptr; return ret; } /* allocate a user handle for a given object */ -user_handle_t alloc_user_handle( void *ptr, enum user_object type ) +user_handle_t alloc_user_handle( void *ptr, enum user_object type, const volatile void *object_shm ) { struct user_handle *entry = alloc_user_entry(); if (!entry) return 0; entry->ptr = ptr; + entry->shm = object_shm; entry->type = type; if (++entry->generation >= 0xffff) entry->generation = 1; return entry_to_handle( entry ); @@ -179,7 +183,7 @@ void free_process_user_handles( struct process *process ) /* allocate an arbitrary user handle */ DECL_HANDLER(alloc_user_handle) { - reply->handle = alloc_user_handle( current->process, USER_CLIENT ); + reply->handle = alloc_user_handle( current->process, USER_CLIENT, NULL ); } @@ -193,3 +197,13 @@ DECL_HANDLER(free_user_handle) else set_error( STATUS_INVALID_HANDLE ); } + +DECL_HANDLER(get_shared_user_object) +{ + struct user_handle *entry; + + if (!(entry = handle_to_entry( req->handle )) || entry->type != req->type || !entry->shm) + set_error( STATUS_INVALID_HANDLE ); + else + reply->locator = get_shared_object_locator( entry->shm ); +} diff --git a/server/user.h b/server/user.h index ce463b9395d..327fc25c427 100644 --- a/server/user.h +++ b/server/user.h @@ -99,7 +99,7 @@ struct desktop /* user handles functions */ -extern user_handle_t alloc_user_handle( void *ptr, enum user_object type ); +extern user_handle_t alloc_user_handle( void *ptr, enum user_object type, const volatile void *object_shm ); extern void *get_user_object( user_handle_t handle, enum user_object type ); extern void *get_user_object_handle( user_handle_t *handle, enum user_object type ); extern user_handle_t get_user_full_handle( user_handle_t handle ); diff --git a/server/window.c b/server/window.c index 9357536aebf..f69e79b5d64 100644 --- a/server/window.c +++ b/server/window.c @@ -672,7 +672,7 @@ static struct window *create_window( struct window *parent, struct window *owner list_init( &win->unlinked ); if (!(win->shared = alloc_shared_object())) goto failed; - if (!(handle = alloc_user_handle( win, USER_WINDOW ))) goto failed; + if (!(handle = alloc_user_handle( win, USER_WINDOW, win->shared ))) goto failed; win->last_active = win->shared->handle; SHARED_WRITE_BEGIN( win->shared, window_shm_t ) @@ -2345,7 +2345,6 @@ DECL_HANDLER(get_window_info) if (!win) return; - reply->full_handle = win->shared->handle; reply->last_active = win->shared->handle; reply->is_unicode = win->is_unicode; reply->dpi_context = win->dpi_context; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7512
Jinoh Kang (@iamahuman) commented about dlls/win32u/winstation.c:
return status; }
+#define NB_USER_HANDLES ((LAST_USER_HANDLE - FIRST_USER_HANDLE + 1) >> 1) +#define USER_HANDLE_TO_INDEX(handle) ((LOWORD(handle) - FIRST_USER_HANDLE) >> 1) + +static struct shared_user_object_cache user_objects[NB_USER_HANDLES];
This field alone is taking up ~0.5GiB of virtual memory, per-process. We already have `user_handles`, can we somehow reuse that? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_96972
On Thu Mar 6 14:12:09 2025 +0000, Jinoh Kang wrote:
This field alone is taking up ~0.5GiB of virtual memory, per-process. We already have `user_handles`, can we somehow reuse that? I guess that would involve nontrivial refactoring.
I'm honestly not sure how much of an issue this would be in practice. It would largely depend on handle allocation patterns, and whether we still care about old wow64/32bit mode or not. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_96973
On Thu Mar 6 14:19:59 2025 +0000, Jinoh Kang wrote:
I guess that would involve nontrivial refactoring. I'm honestly not sure how much of an issue this would be in practice. It would largely depend on handle allocation patterns, and whether we still care about old wow64/32bit mode or not. You mean 0.5MiB? Which doesn't seem exactly bad.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_96974
On Thu Mar 6 14:27:52 2025 +0000, Rémi Bernon wrote:
You mean 0.5MiB? Which doesn't seem exactly bad. Okay, well, can I delete my embarrassment if no one minds?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_96975
On Thu Mar 6 20:56:02 2025 +0000, Jinoh Kang wrote:
Okay, well, can I delete my embarrassment if no one minds? In ntdll we deal with this by having two levels of access and allocating one page at a time. Should we do that here as well?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97007
On Thu Mar 6 20:56:02 2025 +0000, Elizabeth Figura wrote:
In ntdll we deal with this by having two levels of access and allocating one page at a time. Should we do that here as well? Is it really worth the trouble? The other array of object handles in win32u is also just static array. Should we worry for a hundred of KB on the unix side?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97011
On Thu Mar 6 21:19:16 2025 +0000, Rémi Bernon wrote:
Is it really worth the trouble? The other array of object handles in win32u is also just static array. Should we worry for a hundred of KB on the unix side? If memory usage is going to be a recurring problem, perhaps Wine should define a concrete upper bound and test for the working set.
Also note that, when completely filled, two-level table ends up having more overhead than single level. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97027
Windows has a similar shared memory for user handles, see https://gitlab.winehq.org/jacek/wine/-/commit/3d0ba4ed7873d1b8b4da6e7e3fea9b... (and the https://gitlab.winehq.org/jacek/wine/-/commits/user-handles branch for a draft of the implementation). In case of Windows, it allows skipping syscall overhead. I guess that's why there are no win32u exports for things like `GetWindowLong` or `IsWindow`. While it's probably fine to ignore the user space (at least in the initial version), basing it on the locator that's accessible only via server calls is not necessarily the best approach. The user handle should be enough to locate shm entries. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97134
On Sat Mar 8 03:44:44 2025 +0000, Jacek Caban wrote:
Windows has a similar shared memory for user handles, see https://gitlab.winehq.org/jacek/wine/-/commit/3d0ba4ed7873d1b8b4da6e7e3fea9b... (and the https://gitlab.winehq.org/jacek/wine/-/commits/user-handles branch for a draft of the implementation). In case of Windows, it allows skipping syscall overhead. I guess that's why there are no win32u exports for things like `GetWindowLong` or `IsWindow`. While it's probably fine to ignore the user space (at least in the initial version), basing it on the locator that's accessible only via server calls is not necessarily the best approach. The user handle should be enough to locate shm entries. A locator essentially serves as the bottom part of a `USER_HANDLE_ENTRY`, so I guess it should be easy to replace when the user handle table lands in the session shmem.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97166
On Sat Mar 8 03:44:44 2025 +0000, Jinoh Kang wrote:
A locator essentially serves as the bottom part of a `USER_HANDLE_ENTRY`, so I guess it should be easy to replace when the user handle table lands in the session shmem. (For objects w/o a user handle, the locator itself won't go away unless we figure out the format for them)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97167
On Sat Mar 8 03:53:34 2025 +0000, Jinoh Kang wrote:
(For objects w/o a user handle, the locator itself won't go away unless we figure out the format Windows uses for them) The question is also whether we want to use that windows internal structure for our implementation. I'm not convinced that it is a good idea. The structure is completely undocumented, it is different between 32bit and 64bit kernels, and has changed already since Windows 8.1 as tests can attest.
If we ever need to expose a compatible handle table in the shared memory, which I doubt given the above, I believe we can always emulate it as needed. If we want to use out shared memory objects directly on the PE side, we can always expose locator query functions through NtUser calls, like gSharedInfo would require to be initialized anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97172
On Sat Mar 8 09:11:48 2025 +0000, Rémi Bernon wrote:
The question is also whether we want to use that windows internal structure for our implementation. I'm not convinced that it is a good idea. The structure is completely undocumented, it is different between 32bit and 64bit kernels, and has changed already since Windows 8.1 as tests can attest (W81 / W10 1511 / W10 21H2 all have different handle entry layout). If we ever need to expose a compatible handle table in the shared memory, which I doubt given the above, I believe we can always emulate it as needed, from our own shared memory kept separately. If we want to use our shared memory objects directly on the PE side, we can always expose locator query functions through NtUser calls, like gSharedInfo would require to be initialized anyway. I think Jacek was talking about making the user object cache itself shared, rather than emulating gSharedInfo
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97175
On Sat Mar 8 09:17:58 2025 +0000, Jinoh Kang wrote:
I think Jacek was talking about making the user object cache itself shared, rather than emulating gSharedInfo I don't understand, what's the difference? The user object cache is the gSharedInfo handle table isn't it? It doesn't have a stable layout.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97176
On Sat Mar 8 09:21:05 2025 +0000, Rémi Bernon wrote:
I don't understand, what's the difference? The user object cache is the gSharedInfo handle table isn't it? It doesn't have a stable layout. We can manage a table of locators `obj_locator_t user_objects[MAX_USER_HANDLES]` and make it shared. That way no extra server call is needed to map HWND to obj\_locator\_t. Just an array of shared offset works too, if we reuse HWND for the object ID.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97177
On Sat Mar 8 09:36:25 2025 +0000, Jinoh Kang wrote:
We can manage a table of locators `obj_locator_t user_objects[MAX_USER_HANDLES]` and make it shared. That way no extra server call is needed to map HWND to obj\_locator\_t. Just an array of shared offset works too, if we reuse HWND for the object ID. That is not to say I have an opinion about this, strong or otherwise; I haven't reviewed the MR anyway. That said it could eliminate some extra `top_locator` fields and so on.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97178
On Sat Mar 8 09:48:57 2025 +0000, Jinoh Kang wrote:
That is not to say I have an opinion about this, strong or otherwise; I haven't reviewed the MR anyway. That said it could eliminate some extra `top_locator` fields and so on. Yes, I was mostly thinking about how we approach the problem, not `gSharedInfo` itself.
User handles are different than NT handles. They don't have a counterpart of `DuplicateHandle`, have the same value among processes, and objects' life time is tied to handle life time. That makes a shared handle table pretty straightforward. All we need is to be able to validate the handle and get a shm offset from it. That would eliminate the need for `get_shared_user_object` entirely and make using "locator" infrastructure an unnecessary complication. The layout of the table is a secondary concern, but with a shared handle table in place, making it compatible with modern 64-bit Windows version seems trivial. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97245
On Sun Mar 9 11:10:04 2025 +0000, Jacek Caban wrote:
Yes, I was mostly thinking about how we approach the problem, not `gSharedInfo` itself. User handles are different than NT handles. They don't have a counterpart of `DuplicateHandle`, have the same value among processes, and objects' life time is tied to handle life time. That makes a shared handle table pretty straightforward. All we need is to be able to validate the handle and get a shm offset from it. That would eliminate the need for `get_shared_user_object` entirely and make using "locator" infrastructure an unnecessary complication. The layout of the table is a secondary concern, but with a shared handle table in place, making it compatible with modern 64-bit Windows version seems trivial. The locator infrastructure is not something we can really get away from by using user handles.
You can name it differently, but you'll still have to have an object expected id (name it handle, object_id or anything) and its offset. Reading an object data will always require a similar read loop like we have for other objects, to ensure consistency of the data that's been read, checking the object id before and after, and retry locating the object if it was invalidated. I don't really see the point, and I don't think it's a good idea to reimplement another similar data consistency mechanism just for user objects, that would use user handles instead of `object_id_t` like existing objects. Yes, we can put the handle table in shm, put some of the information that seems to be there on Windows gSharedInfo, perhaps keep server and client pointers there[^1], but I think we should use obj_locator still to locate and identify objects in a stronger and more consistent way than user handles only would allow. Our object ids are 64bit and increasing on any change, user handles are only 32bit and more subject to collisions or ABA races, and it doesn't seem better to use them for consistency. To me, the unnecessary complication is to try to put another shared object model on top / aside the existing one when there no actual need for it. And making the handle layout table compatible looks far from trivial, or even useful, when it seem to regularly change. [^1] The entry layout in your branch doesn't seem to match what I'm seeing on latest Windows 10, there's PID, object type and handle generation maybe what seems to be an offset at a different offset but that's about it, no obvious client/server pointers, no PID either. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97257
On Mon Mar 10 11:02:04 2025 +0000, Rémi Bernon wrote:
The locator infrastructure is not something we can really get away from by using user handles. You can name it differently, but you'll still have to have an object expected id (name it handle, `object_id_t` or anything) and its offset. Reading an object data will always require a similar read loop like we have for other objects, to ensure consistency of the data that's been read, checking the object id before and after, and retry locating the object if it was invalidated. I don't really see the point, and I don't think it's a good idea to reimplement another similar data consistency mechanism just for user objects, that would use user handles instead of `object_id_t` like existing objects. Yes, we can put the handle table in shm, put some of the information that seems to be there on Windows gSharedInfo, perhaps keep server and client pointers there[^1], but I think we should use obj_locator still to locate and identify objects in a stronger and more consistent way than user handles only would allow. Our object ids are 64bit and increasing on any change, user handles are only 32bit and more subject to collisions or ABA races, and it doesn't seem better to use them for consistency. To me, the unnecessary complication is to try to put another shared object model on top / aside the existing one when there no actual need for it. And making the handle layout table compatible looks far from trivial, or even useful, when it seem to regularly change. [^1]: The entry layout in your branch doesn't seem to match what I'm seeing on latest Windows 10, there's TID, object type and handle generation, and maybe what seems to be an offset at a different position but that's about it, no obvious client/server pointers, no PID either. I think that one of more interesting optimizations is `IsWindow`, for which we need shared handle table and handle validation on top of that. Your approach will always need a server call for invalid handles.
You can name it differently, but you'll still have to have an object expected id (name it handle, `object_id_t` or anything) and its offset.
No, you don't need any expected id. As I said, unlike with NT handles, lifetime of the handle is the same lifetime of its underlying object. Handle validation is enough and it's something that would be good to have anyway (see above). We'd indeed need an offset, but that's it.
The entry layout in your branch doesn't seem to match what I'm seeing on latest Windows 10, there's TID, object type and handle generation, and maybe what seems to be an offset at a different position but that's about it, no obvious client/server pointers, no PID either.
The offset is all you need. Whatever is missing in the handle table could be a part of the struct referenced by that offset. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97699
On Thu Mar 13 14:24:27 2025 +0000, Jacek Caban wrote:
I think that one of more interesting optimizations is `IsWindow`, for which we need shared handle table and handle validation on top of that. Your approach will always need a server call for invalid handles.
You can name it differently, but you'll still have to have an object expected id (name it handle, `object_id_t` or anything) and its offset. No, you don't need any expected id. As I said, unlike with NT handles, lifetime of the handle is the same lifetime of its underlying object. Handle validation is enough and it's something that would be good to have anyway (see above). We'd indeed need an offset, but that's it. The entry layout in your branch doesn't seem to match what I'm seeing on latest Windows 10, there's TID, object type and handle generation, and maybe what seems to be an offset at a different position but that's about it, no obvious client/server pointers, no PID either. The offset is all you need. Whatever is missing in the handle table could be a part of the struct referenced by that offset. The offset is ephemeral by nature: the struct referenced by the offset can always be freed and reused by another object at any time, even during a read from a client. We still need a way to verify that the "read critical section" is consistently reading from the same USER object throughout the duration of the critical section, instead of some dangling/freed offset due to an abrupt replacement/destruction of the object.
One obvious way to resolve is to make the USER handle (itself consisting of index + generation) a part of the struct referenced by that offset. This is what I assume Rémi is referring to as the "ID". -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97700
On Thu Mar 13 14:44:59 2025 +0000, Jinoh Kang wrote:
The offset is ephemeral by nature: the struct referenced by the offset can always be freed and reused by another object at any time, even during a read from a client. We still need a way to verify that the "read critical section" is consistently reading from the same USER object throughout the duration of the critical section, instead of some dangling/freed offset due to an abrupt replacement/destruction of the object. One obvious way to resolve is to make the USER handle (itself consisting of index + generation) a part of the struct referenced by that offset. This is what I assume Rémi is referring to as the "ID". We may just validate the handle again after reading.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97701
On Thu Mar 13 14:47:06 2025 +0000, Jacek Caban wrote:
We may just validate the handle again after reading. Fwiw I'm not arguing against a shared handle table, it's not implemented in this version but it's possible to have without much changes. Having a shared handle table is orthogonal to how actual objects are identified/validated.
Let's consider this scenario: 1) Thread A reads entry for handle 123, get offset 456 2) Thread B updates handle 123 object which gets moved to offset 789 for whatever reason (needs growing, etc) 3) Thread A access object data at offset 456, reads invalid data 4) Thread B updates handle 123 object which gets moved back to offset 456 for some other reason In both cases thread A needs to know that the data it has read is invalid. It's not possible with offset only, unless we enforce additional restriction on the objects. In addition, even with additional restrictions, the validation needs to be done by reading the shared handle table again, checking that the handle is still valid and that the offset is still the same. This is different from the existing object validation mechanism, and requires writing another piece of logic for user objects. Instead, using the same underlying shared object infrastructure for user objects, we have a validation mechanism that works in every scenario, and is consistent with other objects. Reading id+offset from the shared handle table is enough to validate objects without having to check again the table. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97702
Thread B updates handle 123 object which gets moved to offset 789 for whatever reason (needs growing, etc)
Replacing shared object placement seems redundant. Why would it need to grow? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97704
On Thu Mar 13 14:51:47 2025 +0000, Jacek Caban wrote:
Thread B updates handle 123 object which gets moved to offset 789 for whatever reason (needs growing, etc) Replacing shared object placement seems redundant. Why would it need to grow? Why would it not? NtUserSetCursorIconData for instance seems to be able to add a variable number of frames to an existing HCURSOR object.
Requiring object size to be immutable is quite a strong requirement which isn't necessary with the existing object validation mechanism. I really don't see the point in implementing a different object validation mechanism that support only a subset of what the existing one can do. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97705
I really don't see the point in implementing a different object validation mechanism that support only a subset of what the existing one can do.
`IsWindow` would need it anyway. If we agree on that, `get_shared_user_object` is simply a step in a wrong direction. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512#note_97707
This merge request was closed by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7512
participants (4)
-
Elizabeth Figura (＠zfigura) -
Jacek Caban (＠jacek)
-
Jinoh Kang (＠iamahuman)
-
Rémi Bernon