[PATCH v32 0/9] MR3103: server: Create and use a desktop shared mapping for GetCursorPos.
First part of Proton shared memory series. The full branch can be seen at https://gitlab.winehq.org/rbernon/wine/-/commits/mr/shared-memories. -- v32: win32u: Use the desktop shared data for GetCursorPos. server: Move the last cursor time to the desktop session object. server: Move the cursor position to the desktop session object. win32u: Open desktop shared objects from session mapping. server: Return the desktop object info in get_thread_desktop. server: Allocate shared session object for desktops. win32u: Open the global session shared mapping. include: Add ReadNoFence64 inline helpers. server: Create a global session shared mapping. https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/directory.c | 7 +++++++ server/file.h | 19 +++++++++++++++++++ server/mapping.c | 43 +++++++++++++++++++++++++++++++++++++++++++ server/object.c | 13 ++++++++++++- server/object.h | 2 ++ server/protocol.def | 16 ++++++++++++++++ tools/make_requests | 1 + 7 files changed, 100 insertions(+), 1 deletion(-) diff --git a/server/directory.c b/server/directory.c index e521a7b38c9..224bd999219 100644 --- a/server/directory.c +++ b/server/directory.c @@ -439,11 +439,14 @@ void init_directories( struct fd *intl_fd ) /* mappings */ static const WCHAR intlW[] = {'N','l','s','S','e','c','t','i','o','n','L','A','N','G','_','I','N','T','L'}; static const WCHAR user_dataW[] = {'_','_','w','i','n','e','_','u','s','e','r','_','s','h','a','r','e','d','_','d','a','t','a'}; + static const WCHAR sessionW[] = {'_','_','w','i','n','e','_','s','e','s','s','i','o','n'}; static const struct unicode_str intl_str = {intlW, sizeof(intlW)}; static const struct unicode_str user_data_str = {user_dataW, sizeof(user_dataW)}; + static const struct unicode_str session_str = {sessionW, sizeof(sessionW)}; struct directory *dir_driver, *dir_device, *dir_global, *dir_kernel, *dir_nls; struct object *named_pipe_device, *mailslot_device, *null_device; + struct mapping *session_mapping; unsigned int i; root_directory = create_directory( NULL, NULL, OBJ_PERMANENT, HASH_SIZE, NULL ); @@ -491,6 +494,10 @@ void init_directories( struct fd *intl_fd ) release_object( create_user_data_mapping( &dir_kernel->obj, &user_data_str, OBJ_PERMANENT, NULL )); release_object( intl_fd ); + session_mapping = create_session_mapping( &dir_kernel->obj, &session_str, OBJ_PERMANENT, NULL ); + set_session_mapping( session_mapping ); + release_object( session_mapping ); + release_object( named_pipe_device ); release_object( mailslot_device ); release_object( null_device ); diff --git a/server/file.h b/server/file.h index 7f2d1637863..a7397ffd093 100644 --- a/server/file.h +++ b/server/file.h @@ -188,6 +188,25 @@ extern struct mapping *create_fd_mapping( struct object *root, const struct unic unsigned int attr, const struct security_descriptor *sd ); extern struct object *create_user_data_mapping( struct object *root, const struct unicode_str *name, unsigned int attr, const struct security_descriptor *sd ); +extern struct mapping *create_session_mapping( struct object *root, const struct unicode_str *name, + unsigned int attr, const struct security_descriptor *sd ); +extern void set_session_mapping( struct mapping *mapping ); + +#define SHARED_WRITE_BEGIN( object_shm, type ) \ + do { \ + const type *__shared = (object_shm); \ + type *shared = (type *)__shared; \ + session_object_t *__obj = CONTAINING_RECORD( shared, session_object_t, shm ); \ + LONG64 __seq = __obj->seq + 1, __end = __seq + 1; \ + assert( (__seq & 1) != 0 ); \ + __WINE_ATOMIC_STORE_RELEASE( &__obj->seq, &__seq ); \ + do + +#define SHARED_WRITE_END \ + while(0); \ + assert( __seq == __obj->seq ); \ + __WINE_ATOMIC_STORE_RELEASE( &__obj->seq, &__end ); \ + } while(0) /* device functions */ diff --git a/server/mapping.c b/server/mapping.c index ff99b45ce51..79072c8b9c8 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -225,6 +225,14 @@ static const mem_size_t granularity_mask = 0xffff; static struct addr_range ranges32; static struct addr_range ranges64; +struct session +{ + const session_object_t *objects; + unsigned int object_capacity; +}; +static struct mapping *session_mapping; +static struct session session; + #define ROUND_SIZE(size) (((size) + page_mask) & ~page_mask) void init_memory(void) @@ -1256,6 +1264,41 @@ int get_page_size(void) return page_mask + 1; } +struct mapping *create_session_mapping( struct object *root, const struct unicode_str *name, + unsigned int attr, const struct security_descriptor *sd ) +{ + static const unsigned int access = FILE_READ_DATA | FILE_WRITE_DATA; + mem_size_t size = max( sizeof(*session.objects) * 512, 0x10000 ); + + return create_mapping( root, name, attr, size, SEC_COMMIT, 0, access, sd ); +} + +static void mark_session_object_free( const session_object_t *object ) +{ + SHARED_WRITE_BEGIN( &object->shm, object_shm_t ) + { + /* mark the object data as not accessible */ + mark_block_noaccess( (void *)shared, sizeof(*shared) ); + CONTAINING_RECORD( shared, session_object_t, shm )->id = 0; + } + SHARED_WRITE_END; +} + +void set_session_mapping( struct mapping *mapping ) +{ + unsigned int index; + + session.objects = mmap( NULL, mapping->size, PROT_READ | PROT_WRITE, MAP_SHARED, get_unix_fd( mapping->fd ), 0 ); + if (session.objects == MAP_FAILED) return; + + session_mapping = mapping; + session.object_capacity = mapping->size / sizeof(*session.objects); + assert( session.object_capacity != -1 ); + + for (index = 0; index < session.object_capacity; index++) + mark_session_object_free( &session.objects[index] ); +} + struct object *create_user_data_mapping( struct object *root, const struct unicode_str *name, unsigned int attr, const struct security_descriptor *sd ) { diff --git a/server/object.c b/server/object.c index 89e541ffb6b..9204593cb6c 100644 --- a/server/object.c +++ b/server/object.c @@ -102,8 +102,19 @@ void close_objects(void) /*****************************************************************/ +/* mark a block of memory as not accessible for debugging purposes */ +void mark_block_noaccess( void *ptr, size_t size ) +{ + memset( ptr, 0xfe, size ); +#if defined(VALGRIND_MAKE_MEM_NOACCESS) + VALGRIND_DISCARD( VALGRIND_MAKE_MEM_NOACCESS( ptr, size ) ); +#elif defined(VALGRIND_MAKE_NOACCESS) + VALGRIND_DISCARD( VALGRIND_MAKE_NOACCESS( ptr, size ) ); +#endif +} + /* mark a block of memory as uninitialized for debugging purposes */ -static inline void mark_block_uninitialized( void *ptr, size_t size ) +void mark_block_uninitialized( void *ptr, size_t size ) { memset( ptr, 0x55, size ); #if defined(VALGRIND_MAKE_MEM_UNDEFINED) diff --git a/server/object.h b/server/object.h index d4d66536b81..2337ee88231 100644 --- a/server/object.h +++ b/server/object.h @@ -139,6 +139,8 @@ struct wait_queue_entry struct thread_wait *wait; }; +extern void mark_block_noaccess( void *ptr, size_t size ); +extern void mark_block_uninitialized( void *ptr, size_t size ); extern void *mem_alloc( size_t size ) __WINE_ALLOC_SIZE(1) __WINE_DEALLOC(free) __WINE_MALLOC; extern void *memdup( const void *data, size_t len ) __WINE_ALLOC_SIZE(2) __WINE_DEALLOC(free); extern void *alloc_object( const struct object_ops *ops ); diff --git a/server/protocol.def b/server/protocol.def index 55dda2d7636..32642b10c09 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -45,6 +45,7 @@ typedef unsigned __int64 mem_size_t; typedef unsigned __int64 file_pos_t; typedef unsigned __int64 client_ptr_t; typedef unsigned __int64 affinity_t; +typedef unsigned __int64 object_id_t; typedef client_ptr_t mod_handle_t; struct request_header @@ -874,6 +875,21 @@ typedef struct lparam_t info; } cursor_pos_t; +/****************************************************************/ +/* shared session mapping structures */ + +typedef volatile union +{ + char placeholder; +} object_shm_t; + +typedef volatile struct +{ + LONG64 seq; /* sequence number - server updating if (seq & 1) != 0 */ + object_id_t id; /* object unique id, object data is valid if != 0 */ + object_shm_t shm; /* object shared data */ +} session_object_t; + /****************************************************************/ /* Request declarations */ diff --git a/tools/make_requests b/tools/make_requests index e3eaaf45b6f..f22ad279b61 100755 --- a/tools/make_requests +++ b/tools/make_requests @@ -42,6 +42,7 @@ my %formats = "file_pos_t" => [ 8, 8, "&dump_uint64" ], "mem_size_t" => [ 8, 8, "&dump_uint64" ], "affinity_t" => [ 8, 8, "&dump_uint64" ], + "object_id_t" => [ 8, 8, "&dump_uint64" ], "timeout_t" => [ 8, 8, "&dump_timeout" ], "abstime_t" => [ 8, 8, "&dump_abstime" ], "rectangle_t" => [ 16, 4, "&dump_rectangle" ], -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
From: Rémi Bernon <rbernon(a)codeweavers.com> --- include/winnt.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/winnt.h b/include/winnt.h index e7c322fd127..5ea908c97ad 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -7059,11 +7059,14 @@ static FORCEINLINE void MemoryBarrier(void) */ #if _MSC_VER >= 1700 #pragma intrinsic(__iso_volatile_load32) +#pragma intrinsic(__iso_volatile_load64) #pragma intrinsic(__iso_volatile_store32) #define __WINE_LOAD32_NO_FENCE(src) (__iso_volatile_load32(src)) +#define __WINE_LOAD64_NO_FENCE(src) (__iso_volatile_load64(src)) #define __WINE_STORE32_NO_FENCE(dest, value) (__iso_volatile_store32(dest, value)) #else /* _MSC_VER >= 1700 */ #define __WINE_LOAD32_NO_FENCE(src) (*(src)) +#define __WINE_LOAD64_NO_FENCE(src) (*(src)) #define __WINE_STORE32_NO_FENCE(dest, value) ((void)(*(dest) = (value))) #endif /* _MSC_VER >= 1700 */ @@ -7097,6 +7100,12 @@ static FORCEINLINE LONG ReadNoFence( LONG const volatile *src ) return value; } +static FORCEINLINE LONG64 ReadNoFence64( LONG64 const volatile *src ) +{ + LONG64 value = __WINE_LOAD64_NO_FENCE( (__int64 const volatile *)src ); + return value; +} + static FORCEINLINE void WriteRelease( LONG volatile *dest, LONG value ) { __wine_memory_barrier_acq_rel(); @@ -7283,6 +7292,13 @@ static FORCEINLINE LONG ReadNoFence( LONG const volatile *src ) return value; } +static FORCEINLINE LONG64 ReadNoFence64( LONG64 const volatile *src ) +{ + LONG64 value; + __WINE_ATOMIC_LOAD_RELAXED( src, &value ); + return value; +} + static FORCEINLINE void WriteRelease( LONG volatile *dest, LONG value ) { __WINE_ATOMIC_STORE_RELEASE( dest, &value ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/winstation.c | 102 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/dlls/win32u/winstation.c b/dlls/win32u/winstation.c index a3aed32f1da..790b3e8b3c5 100644 --- a/dlls/win32u/winstation.c +++ b/dlls/win32u/winstation.c @@ -22,9 +22,13 @@ #pragma makedep unix #endif +#include <assert.h> +#include <stdarg.h> +#include <stddef.h> +#include <pthread.h> + #include "ntstatus.h" #define WIN32_NO_STATUS -#include <stdarg.h> #include "windef.h" #include "winbase.h" #include "ntuser.h" @@ -40,6 +44,97 @@ WINE_DECLARE_DEBUG_CHANNEL(win); #define DESKTOP_ALL_ACCESS 0x01ff +struct shared_session +{ + LONG ref; + unsigned int object_capacity; + const session_object_t *objects; +}; + +static pthread_mutex_t session_lock = PTHREAD_MUTEX_INITIALIZER; +static struct shared_session *shared_session; + +static struct shared_session *shared_session_acquire( struct shared_session *session ) +{ + int ref = InterlockedIncrement( &session->ref ); + TRACE( "session %p incrementing ref to %d\n", session, ref ); + return session; +} + +static void shared_session_release( struct shared_session *session ) +{ + int ref = InterlockedDecrement( &session->ref ); + TRACE( "session %p decrementing ref to %d\n", session, ref ); + if (!ref) + { + NtUnmapViewOfSection( GetCurrentProcess(), (void *)session->objects ); + free( session ); + } +} + +static NTSTATUS open_shared_session_section( struct shared_session *session ) +{ + static const WCHAR nameW[] = + { + '\\','K','e','r','n','e','l','O','b','j','e','c','t','s','\\', + '_','_','w','i','n','e','_','s','e','s','s','i','o','n',0 + }; + UNICODE_STRING name = RTL_CONSTANT_STRING( nameW ); + OBJECT_ATTRIBUTES attr; + unsigned int status; + SIZE_T size = 0; + HANDLE handle; + + InitializeObjectAttributes( &attr, &name, 0, NULL, NULL ); + if ((status = NtOpenSection( &handle, SECTION_MAP_READ, &attr ))) return status; + if (!(status = NtMapViewOfSection( handle, GetCurrentProcess(), (void **)&session->objects, 0, 0, + NULL, &size, ViewUnmap, 0, PAGE_READONLY ))) + { + session->object_capacity = size / sizeof(*session->objects); + assert( session->object_capacity != -1 ); + } + + NtClose( handle ); + return status; +} + +static NTSTATUS get_shared_session( BOOL force, struct shared_session **ret ) +{ + TRACE( "force %u\n", force ); + + pthread_mutex_lock( &session_lock ); + + if (force || !shared_session) + { + struct shared_session *session; + unsigned int status; + + if (!(session = calloc( 1, sizeof(*session) ))) + { + pthread_mutex_unlock( &session_lock ); + return STATUS_NO_MEMORY; + } + session->ref = 1; + + if ((status = open_shared_session_section( session ))) + { + pthread_mutex_unlock( &session_lock ); + ERR( "Failed to open shared session mapping, status %#x\n", status ); + free( session ); + return STATUS_UNSUCCESSFUL; + } + + if (shared_session) shared_session_release( shared_session ); + shared_session = session; + } + + *ret = shared_session_acquire( shared_session ); + + pthread_mutex_unlock( &session_lock ); + + return STATUS_SUCCESS; +} + BOOL is_virtual_desktop(void) { HANDLE desktop = NtUserGetThreadDesktop( GetCurrentThreadId() ); @@ -630,12 +725,17 @@ void winstation_init(void) { RTL_USER_PROCESS_PARAMETERS *params = NtCurrentTeb()->Peb->ProcessParameters; WCHAR *winstation = NULL, *desktop = NULL, *buffer = NULL; + struct shared_session *session; HANDLE handle, dir = NULL; OBJECT_ATTRIBUTES attr; UNICODE_STRING str; + NTSTATUS status; static const WCHAR winsta0[] = {'W','i','n','S','t','a','0',0}; + if (!(status = get_shared_session( FALSE, &session ))) + shared_session_release( session ); + if (params->Desktop.Length) { buffer = malloc( params->Desktop.Length + sizeof(WCHAR) ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/file.h | 4 +++ server/mapping.c | 77 +++++++++++++++++++++++++++++++++++++++++++++ server/protocol.def | 7 ++++- server/user.h | 1 + server/winstation.c | 8 +++++ 5 files changed, 96 insertions(+), 1 deletion(-) diff --git a/server/file.h b/server/file.h index a7397ffd093..691487179aa 100644 --- a/server/file.h +++ b/server/file.h @@ -192,6 +192,10 @@ extern struct mapping *create_session_mapping( struct object *root, const struct unsigned int attr, const struct security_descriptor *sd ); extern void set_session_mapping( struct mapping *mapping ); +extern unsigned int alloc_shared_object(void); +extern void free_shared_object( unsigned int index ); +extern const desktop_shm_t *get_shared_desktop( unsigned int index ); + #define SHARED_WRITE_BEGIN( object_shm, type ) \ do { \ const type *__shared = (object_shm); \ diff --git a/server/mapping.c b/server/mapping.c index 79072c8b9c8..218c9456d4b 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -229,6 +229,8 @@ struct session { const session_object_t *objects; unsigned int object_capacity; + unsigned int last_object_index; + object_id_t last_object_id; }; static struct mapping *session_mapping; static struct session session; @@ -1294,11 +1296,86 @@ void set_session_mapping( struct mapping *mapping ) session_mapping = mapping; session.object_capacity = mapping->size / sizeof(*session.objects); assert( session.object_capacity != -1 ); + session.last_object_index = -1; for (index = 0; index < session.object_capacity; index++) mark_session_object_free( &session.objects[index] ); } +static int grow_session_mapping(void) +{ + unsigned int index, capacity; + mem_size_t size; + int unix_fd; + void *tmp; + + capacity = session.object_capacity * 3 / 2; + size = sizeof(*session.objects) * capacity; + size = (size + page_mask) & ~((mem_size_t)page_mask); + capacity = size / sizeof(*session.objects); + assert( capacity > session.object_capacity ); + + unix_fd = get_unix_fd( session_mapping->fd ); + if (!grow_file( unix_fd, size )) return -1; + + if ((tmp = mmap( NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, unix_fd, 0 )) == MAP_FAILED) + { + file_set_error(); + return -1; + } + munmap( (void *)session.objects, session_mapping->size ); + session.objects = tmp; + + for (index = session.object_capacity; index < capacity; index++) + mark_session_object_free( &session.objects[index] ); + + session_mapping->size = size; + session.object_capacity = capacity; + assert( session.object_capacity != -1 ); + + return 0; +} + +unsigned int alloc_shared_object(void) +{ + unsigned int index, offset = session.last_object_index + 1, capacity = session.object_capacity; + + for (index = offset; index != offset + capacity; index++) + if (!session.objects[index % capacity].id) + break; + if (index != offset + capacity) index %= capacity; + else + { + if (grow_session_mapping()) return -1; + index = capacity; + } + + assert( index < session.object_capacity ); + session.last_object_index = index; + + SHARED_WRITE_BEGIN( &session.objects[index].shm, object_shm_t ) + { + /* mark the object data as uninitialized */ + mark_block_uninitialized( (void *)shared, sizeof(*shared) ); + CONTAINING_RECORD( shared, session_object_t, shm )->id = ++session.last_object_id; + } + SHARED_WRITE_END; + + return index; +} + +void free_shared_object( unsigned int index ) +{ + if (index >= session.object_capacity) return; + mark_session_object_free( &session.objects[index] ); +} + +const desktop_shm_t *get_shared_desktop( unsigned int index ) +{ + if (index >= session.object_capacity) return NULL; + return &session.objects[index].shm.desktop; +} + struct object *create_user_data_mapping( struct object *root, const struct unicode_str *name, unsigned int attr, const struct security_descriptor *sd ) { diff --git a/server/protocol.def b/server/protocol.def index 32642b10c09..b2ebf3c2b3a 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -878,9 +878,14 @@ typedef struct /****************************************************************/ /* shared session mapping structures */ -typedef volatile union +typedef volatile struct { char placeholder; +} desktop_shm_t; + +typedef volatile union +{ + desktop_shm_t desktop; } object_shm_t; typedef volatile struct diff --git a/server/user.h b/server/user.h index d805a179d16..6079ab92451 100644 --- a/server/user.h +++ b/server/user.h @@ -82,6 +82,7 @@ struct desktop unsigned int users; /* processes and threads using this desktop */ struct global_cursor cursor; /* global cursor information */ unsigned char keystate[256]; /* asynchronous key state */ + unsigned int session_index; /* desktop index in session shared memory */ }; /* user handles functions */ diff --git a/server/winstation.c b/server/winstation.c index 80126ad5d60..167ac8aeb62 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -297,6 +297,13 @@ static struct desktop *create_desktop( const struct unicode_str *name, unsigned list_add_tail( &winstation->desktops, &desktop->entry ); list_init( &desktop->hotkeys ); list_init( &desktop->pointers ); + desktop->session_index = alloc_shared_object(); + + if (!get_shared_desktop( desktop->session_index )) + { + release_object( desktop ); + return NULL; + } } else { @@ -366,6 +373,7 @@ static void desktop_destroy( struct object *obj ) if (desktop->global_hooks) release_object( desktop->global_hooks ); if (desktop->close_timeout) remove_timeout_user( desktop->close_timeout ); release_object( desktop->winstation ); + free_shared_object( desktop->session_index ); } /* retrieve the thread desktop, checking the handle access rights */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/file.h | 1 + server/mapping.c | 7 +++++++ server/protocol.def | 10 +++++++++- server/trace.c | 7 +++++++ server/winstation.c | 11 +++++++++++ tools/make_requests | 1 + 6 files changed, 36 insertions(+), 1 deletion(-) diff --git a/server/file.h b/server/file.h index 691487179aa..0242fb3b3e7 100644 --- a/server/file.h +++ b/server/file.h @@ -194,6 +194,7 @@ extern void set_session_mapping( struct mapping *mapping ); extern unsigned int alloc_shared_object(void); extern void free_shared_object( unsigned int index ); +extern obj_locator_t get_session_object_locator( unsigned int index ); extern const desktop_shm_t *get_shared_desktop( unsigned int index ); #define SHARED_WRITE_BEGIN( object_shm, type ) \ diff --git a/server/mapping.c b/server/mapping.c index 218c9456d4b..3937817d264 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -1370,6 +1370,13 @@ void free_shared_object( unsigned int index ) mark_session_object_free( &session.objects[index] ); } +obj_locator_t get_session_object_locator( unsigned int index ) +{ + obj_locator_t locator = {.index = index}; + if (index < session.object_capacity) locator.id = session.objects[index].id; + return locator; +} + const desktop_shm_t *get_shared_desktop( unsigned int index ) { if (index >= session.object_capacity) return NULL; diff --git a/server/protocol.def b/server/protocol.def index b2ebf3c2b3a..3206c7909ae 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -895,6 +895,13 @@ typedef volatile struct object_shm_t shm; /* object shared data */ } session_object_t; +typedef struct +{ + object_id_t id; /* object unique id, object data is valid if != 0 */ + unsigned int index; /* index of the object in session shared memory */ + int __pad; +} obj_locator_t; + /****************************************************************/ /* Request declarations */ @@ -2805,7 +2812,8 @@ enum coords_relative @REQ(get_thread_desktop) thread_id_t tid; /* thread id */ @REPLY - obj_handle_t handle; /* handle to the desktop */ + obj_handle_t handle; /* handle to the desktop */ + obj_locator_t locator; /* locator for the shared session object */ @END diff --git a/server/trace.c b/server/trace.c index efb4bb03c4a..925c70e865f 100644 --- a/server/trace.c +++ b/server/trace.c @@ -467,6 +467,13 @@ static void dump_hw_input( const char *prefix, const hw_input_t *input ) } } +static void dump_obj_locator( const char *prefix, const obj_locator_t *locator ) +{ + fprintf( stderr, "%s{", prefix ); + dump_uint64( "id=", &locator->id ); + fprintf( stderr, ",index=%u}", locator->index ); +} + static void dump_luid( const char *prefix, const struct luid *luid ) { fprintf( stderr, "%s%d.%u", prefix, luid->high_part, luid->low_part ); diff --git a/server/winstation.c b/server/winstation.c index 167ac8aeb62..6c47811c6f6 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -741,10 +741,21 @@ DECL_HANDLER(close_desktop) /* get the thread current desktop */ DECL_HANDLER(get_thread_desktop) { + struct desktop *desktop; struct thread *thread; + reply->locator.id = 0; + if (!(thread = get_thread_from_id( req->tid ))) return; reply->handle = thread->desktop; + + if (!(desktop = get_thread_desktop( thread, 0 ))) clear_error(); + else + { + reply->locator = get_session_object_locator( desktop->session_index ); + release_object( desktop ); + } + release_object( thread ); } diff --git a/tools/make_requests b/tools/make_requests index f22ad279b61..3809c51dc25 100755 --- a/tools/make_requests +++ b/tools/make_requests @@ -53,6 +53,7 @@ my %formats = "generic_map_t" => [ 16, 4, "&dump_generic_map" ], "ioctl_code_t" => [ 4, 4, "&dump_ioctl_code" ], "hw_input_t" => [ 40, 8, "&dump_hw_input" ], + "obj_locator_t" => [ 16, 8, "&dump_obj_locator" ], # varargs-only structures "apc_call_t" => [ 64, 8 ], "context_t" => [ 1720, 8 ], -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/ntuser_private.h | 1 + dlls/win32u/sysparams.c | 1 + dlls/win32u/win32u_private.h | 19 +++++ dlls/win32u/winstation.c | 144 +++++++++++++++++++++++++++++++++-- 4 files changed, 160 insertions(+), 5 deletions(-) diff --git a/dlls/win32u/ntuser_private.h b/dlls/win32u/ntuser_private.h index bb2169998b6..c0027f44c2f 100644 --- a/dlls/win32u/ntuser_private.h +++ b/dlls/win32u/ntuser_private.h @@ -127,6 +127,7 @@ struct user_thread_info UINT spy_indent; /* Current spy indent */ BOOL clipping_cursor; /* thread is currently clipping */ DWORD clipping_reset; /* time when clipping was last reset */ + struct session_thread_data *session_data; /* shared session thread data */ }; C_ASSERT( sizeof(struct user_thread_info) <= sizeof(((TEB *)0)->Win32ClientInfo) ); diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 26bddad7b59..899722b9ee1 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -6210,6 +6210,7 @@ static void thread_detach(void) destroy_thread_windows(); cleanup_imm_thread(); NtClose( thread_info->server_queue ); + free( thread_info->session_data ); exiting_thread_id = 0; } diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index ff861db4dd9..0c800e4d10e 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -194,6 +194,25 @@ extern void user_check_not_lock(void); extern BOOL get_vulkan_uuid_from_luid( const LUID *luid, GUID *uuid ); /* winstation.c */ + +struct shared_session; + +struct object_lock +{ + UINT64 id; + UINT64 seq; + struct shared_session *session; /* only valid when locked */ +}; +#define OBJECT_LOCK_INIT {0} + +/* Get shared session object's data pointer, must be called in a loop while STATUS_PENDING + * is returned, lock must be initialized with OBJECT_LOCK_INIT. + * + * The data read from the objects may be transient and no logic should be executed based + * on it, within the loop, or after, unless the function has returned STATUS_SUCCESS. + */ +extern NTSTATUS get_shared_desktop( struct object_lock *lock, const desktop_shm_t **desktop_shm ); + extern BOOL is_virtual_desktop(void); /* window.c */ diff --git a/dlls/win32u/winstation.c b/dlls/win32u/winstation.c index 790b3e8b3c5..5abbdd6e067 100644 --- a/dlls/win32u/winstation.c +++ b/dlls/win32u/winstation.c @@ -25,6 +25,7 @@ #include <assert.h> #include <stdarg.h> #include <stddef.h> + #include <pthread.h> #include "ntstatus.h" @@ -44,6 +45,11 @@ WINE_DECLARE_DEBUG_CHANNEL(win); #define DESKTOP_ALL_ACCESS 0x01ff +struct session_thread_data +{ + obj_locator_t shared_desktop; /* thread desktop shared session object locator */ +}; + struct shared_session { LONG ref; @@ -54,6 +60,32 @@ struct shared_session static pthread_mutex_t session_lock = PTHREAD_MUTEX_INITIALIZER; static struct shared_session *shared_session; +static struct session_thread_data *get_session_thread_data(void) +{ + struct user_thread_info *thread_info = get_user_thread_info(); + if (!thread_info->session_data) thread_info->session_data = calloc(1, sizeof(*thread_info->session_data)); + return thread_info->session_data; +} + +#if defined(__i386__) || defined(__x86_64__) +/* this prevents compilers from incorrectly reordering non-volatile reads (e.g., memcpy) from shared memory */ +#define __SHARED_READ_FENCE do { __asm__ __volatile__( "" ::: "memory" ); } while (0) +#else +#define __SHARED_READ_FENCE __atomic_thread_fence( __ATOMIC_ACQUIRE ) +#endif + +static void session_object_acquire_seqlock( const session_object_t *object, UINT64 *seq ) +{ + while ((*seq = ReadNoFence64( &object->seq )) & 1) YieldProcessor(); + __SHARED_READ_FENCE; +} + +static BOOL session_object_release_seqlock( const session_object_t *object, UINT64 seq ) +{ + __SHARED_READ_FENCE; + return ReadNoFence64( &object->seq ) == seq; +} + static struct shared_session *shared_session_acquire( struct shared_session *session ) { int ref = InterlockedIncrement( &session->ref ); @@ -135,6 +167,104 @@ static NTSTATUS get_shared_session( BOOL force, struct shared_session **ret ) return STATUS_SUCCESS; } +enum object_type +{ + OBJECT_TYPE_DESKTOP = 1, +}; + +static NTSTATUS get_thread_session_object_locator( UINT tid, enum object_type type, obj_locator_t *locator ) +{ + NTSTATUS status; + + TRACE( "tid %04x, type %u, locator %p\n", tid, type, locator ); + + switch (type) + { + case OBJECT_TYPE_DESKTOP: + SERVER_START_REQ( get_thread_desktop ) + { + req->tid = tid; + if (!(status = wine_server_call( req ))) + { + if (!reply->locator.id) status = STATUS_INVALID_HANDLE; + else *locator = reply->locator; + } + } + SERVER_END_REQ; + break; + default: + ERR( "Invalid session object type %u\n", type ); + return STATUS_INVALID_PARAMETER; + } + + return status; +} + +/* return a locked session object for a thread id and type */ +static NTSTATUS get_thread_session_object( UINT tid, enum object_type type, obj_locator_t *locator, + struct object_lock *lock, const object_shm_t **object_shm ) +{ + struct shared_session *session; + BOOL session_valid = TRUE; + NTSTATUS status; + + assert( !lock->id || *object_shm ); + + TRACE( "tid %04x, type %u, locator %p, lock %p, object_shm %p\n", tid, type, locator, lock, object_shm ); + + if (lock->id) + { + /* lock was previously acquired, finish reading and check data consistency */ + const session_object_t *object = CONTAINING_RECORD( *object_shm, session_object_t, shm ); + BOOL valid = lock->id == object->id; + + if (!session_object_release_seqlock( object, lock->seq )) + { + /* retry if seq doesn't match, wineserver has written to it */ + session_object_acquire_seqlock( object, &lock->seq ); + return STATUS_PENDING; + } + + shared_session_release( lock->session ); + memset( lock, 0, sizeof(*lock) ); + + if (valid) return STATUS_SUCCESS; + locator->id = 0; /* invalidate locator if the lock was abandoned due to id mismatch */ + } + + while (!(status = get_shared_session( !session_valid, &session ))) + { + if (!locator->id && (status = get_thread_session_object_locator( tid, type, locator ))) break; + if ((session_valid = locator->index < session->object_capacity)) + { + const session_object_t *object = &session->objects[locator->index]; + lock->id = locator->id; + lock->session = session; + *object_shm = &object->shm; + session_object_acquire_seqlock( object, &lock->seq ); + return STATUS_PENDING; + } + shared_session_release( session ); + memset( locator, 0, sizeof(*locator) ); + } + + WARN( "Failed to find object type %u for thread %04x\n", type, tid ); + if (!status) shared_session_release( session ); + memset( locator, 0, sizeof(*locator) ); + return STATUS_UNSUCCESSFUL; +} + +NTSTATUS get_shared_desktop( struct object_lock *lock, const desktop_shm_t **desktop_shm ) +{ + struct session_thread_data *data = get_session_thread_data(); + obj_locator_t *locator = &data->shared_desktop; + + TRACE( "lock %p, desktop_shm %p\n", lock, desktop_shm ); + + return get_thread_session_object( GetCurrentThreadId(), OBJECT_TYPE_DESKTOP, locator, + lock, (const object_shm_t **)desktop_shm ); +} + BOOL is_virtual_desktop(void) { HANDLE desktop = NtUserGetThreadDesktop( GetCurrentThreadId() ); @@ -356,9 +486,18 @@ BOOL WINAPI NtUserSetThreadDesktop( HDESK handle ) { struct user_thread_info *thread_info = get_user_thread_info(); struct user_key_state_info *key_state_info = thread_info->key_state; + obj_locator_t *locator = &get_session_thread_data()->shared_desktop; + struct object_lock lock = OBJECT_LOCK_INIT; + const desktop_shm_t *desktop_shm; + thread_info->client_info.top_window = 0; thread_info->client_info.msg_window = 0; if (key_state_info) key_state_info->time = 0; + memset( locator, 0, sizeof(*locator) ); + + while (get_shared_desktop( &lock, &desktop_shm ) == STATUS_PENDING) + /* nothing */; + if (was_virtual_desktop != is_virtual_desktop()) update_display_cache( TRUE ); } return ret; @@ -725,17 +864,12 @@ void winstation_init(void) { RTL_USER_PROCESS_PARAMETERS *params = NtCurrentTeb()->Peb->ProcessParameters; WCHAR *winstation = NULL, *desktop = NULL, *buffer = NULL; - struct shared_session *session; HANDLE handle, dir = NULL; OBJECT_ATTRIBUTES attr; UNICODE_STRING str; - NTSTATUS status; static const WCHAR winsta0[] = {'W','i','n','S','t','a','0',0}; - if (!(status = get_shared_session( FALSE, &session ))) - shared_session_release( session ); - if (params->Desktop.Length) { buffer = malloc( params->Desktop.Length + sizeof(WCHAR) ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
From: Rémi Bernon <rbernon(a)codeweavers.com> Based on a patch by Huw Davies <huw(a)codeweavers.com>. --- server/protocol.def | 8 ++++- server/queue.c | 78 ++++++++++++++++++++++++++++----------------- server/user.h | 2 -- server/winstation.c | 11 ++++++- 4 files changed, 65 insertions(+), 34 deletions(-) diff --git a/server/protocol.def b/server/protocol.def index 3206c7909ae..d7b854a6c96 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -878,9 +878,15 @@ typedef struct /****************************************************************/ /* shared session mapping structures */ +struct shared_cursor +{ + int x; /* cursor position */ + int y; +}; + typedef volatile struct { - char placeholder; + struct shared_cursor cursor; /* global cursor information */ } desktop_shm_t; typedef volatile union diff --git a/server/queue.c b/server/queue.c index ed099b3b989..3780e35d83e 100644 --- a/server/queue.c +++ b/server/queue.c @@ -418,6 +418,7 @@ static void queue_cursor_message( struct desktop *desktop, user_handle_t win, un lparam_t wparam, lparam_t lparam ) { static const struct hw_msg_source source = { IMDT_UNAVAILABLE, IMO_SYSTEM }; + const desktop_shm_t *desktop_shm = get_shared_desktop( desktop->session_index ); struct thread_input *input; struct message *msg; @@ -426,8 +427,8 @@ static void queue_cursor_message( struct desktop *desktop, user_handle_t win, un msg->msg = message; msg->wparam = wparam; msg->lparam = lparam; - msg->x = desktop->cursor.x; - msg->y = desktop->cursor.y; + msg->x = desktop_shm->cursor.x; + msg->y = desktop_shm->cursor.y; if (!(msg->win = win) && (input = desktop->foreground_input)) msg->win = input->active; queue_hardware_message( desktop, msg, 1 ); } @@ -465,13 +466,20 @@ static int update_desktop_cursor_window( struct desktop *desktop, user_handle_t static int update_desktop_cursor_pos( struct desktop *desktop, user_handle_t win, int x, int y ) { + const desktop_shm_t *desktop_shm = get_shared_desktop( desktop->session_index ); int updated; x = max( min( x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left ); y = max( min( y, desktop->cursor.clip.bottom - 1 ), desktop->cursor.clip.top ); - updated = (desktop->cursor.x != x || desktop->cursor.y != y); - desktop->cursor.x = x; - desktop->cursor.y = y; + + SHARED_WRITE_BEGIN( desktop_shm, desktop_shm_t ) + { + updated = shared->cursor.x != x || shared->cursor.y != y; + shared->cursor.x = x; + shared->cursor.y = y; + } + SHARED_WRITE_END; + desktop->cursor.last_change = get_tick_count(); if (!win || !is_window_visible( win ) || is_window_transparent( win )) @@ -517,15 +525,17 @@ static void set_cursor_pos( struct desktop *desktop, int x, int y ) static void get_message_defaults( struct msg_queue *queue, int *x, int *y, unsigned int *time ) { struct desktop *desktop = queue->input->desktop; + const desktop_shm_t *desktop_shm = get_shared_desktop( desktop->session_index ); - *x = desktop->cursor.x; - *y = desktop->cursor.y; + *x = desktop_shm->cursor.x; + *y = desktop_shm->cursor.y; *time = get_tick_count(); } /* set the cursor clip rectangle */ void set_clip_rectangle( struct desktop *desktop, const rectangle_t *rect, unsigned int flags, int reset ) { + const desktop_shm_t *desktop_shm = get_shared_desktop( desktop->session_index ); rectangle_t top_rect; unsigned int old_flags; int x, y; @@ -547,9 +557,9 @@ void set_clip_rectangle( struct desktop *desktop, const rectangle_t *rect, unsig desktop->cursor.clip_flags = flags; /* warp the mouse to be inside the clip rect */ - x = max( min( desktop->cursor.x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left ); - y = max( min( desktop->cursor.y, desktop->cursor.clip.bottom - 1 ), desktop->cursor.clip.top ); - if (x != desktop->cursor.x || y != desktop->cursor.y) set_cursor_pos( desktop, x, y ); + x = max( min( desktop_shm->cursor.x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left ); + y = max( min( desktop_shm->cursor.y, desktop->cursor.clip.bottom - 1 ), desktop->cursor.clip.top ); + if (x != desktop_shm->cursor.x || y != desktop_shm->cursor.y) set_cursor_pos( desktop, x, y ); /* request clip cursor rectangle reset to the desktop thread */ if (reset) post_desktop_message( desktop, WM_WINE_CLIPCURSOR, flags, FALSE ); @@ -1673,6 +1683,7 @@ static void prepend_cursor_history( int x, int y, unsigned int time, lparam_t in /* queue a hardware message into a given thread input */ static void queue_hardware_message( struct desktop *desktop, struct message *msg, int always_queue ) { + const desktop_shm_t *desktop_shm = get_shared_desktop( desktop->session_index ); user_handle_t win; struct thread *thread; struct thread_input *input; @@ -1705,8 +1716,8 @@ static void queue_hardware_message( struct desktop *desktop, struct message *msg if (desktop->keystate[VK_XBUTTON2] & 0x80) msg->wparam |= MK_XBUTTON2; break; } - msg->x = desktop->cursor.x; - msg->y = desktop->cursor.y; + msg->x = desktop_shm->cursor.x; + msg->y = desktop_shm->cursor.y; if (msg->win && (thread = get_window_thread( msg->win ))) { @@ -1988,6 +1999,7 @@ static void dispatch_rawinput_message( struct desktop *desktop, struct rawinput_ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, const hw_input_t *input, unsigned int origin, struct msg_queue *sender ) { + const desktop_shm_t *desktop_shm = get_shared_desktop( desktop->session_index ); const struct rawinput_device *device; struct hardware_msg_data *msg_data; struct rawinput_message raw_msg; @@ -2026,19 +2038,19 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons x = input->mouse.x; y = input->mouse.y; if (flags & ~(MOUSEEVENTF_MOVE | MOUSEEVENTF_ABSOLUTE) && - x == desktop->cursor.x && y == desktop->cursor.y) + x == desktop_shm->cursor.x && y == desktop_shm->cursor.y) flags &= ~MOUSEEVENTF_MOVE; } else { - x = desktop->cursor.x + input->mouse.x; - y = desktop->cursor.y + input->mouse.y; + x = desktop_shm->cursor.x + input->mouse.x; + y = desktop_shm->cursor.y + input->mouse.y; } } else { - x = desktop->cursor.x; - y = desktop->cursor.y; + x = desktop_shm->cursor.x; + y = desktop_shm->cursor.y; } if ((foreground = get_foreground_thread( desktop, win ))) @@ -2049,7 +2061,7 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons raw_msg.time = time; raw_msg.message = WM_INPUT; raw_msg.flags = flags; - rawmouse_init( &raw_msg.rawinput, &raw_msg.data.mouse, x - desktop->cursor.x, y - desktop->cursor.y, + rawmouse_init( &raw_msg.rawinput, &raw_msg.data.mouse, x - desktop_shm->cursor.x, y - desktop_shm->cursor.y, raw_msg.flags, input->mouse.data, input->mouse.info ); dispatch_rawinput_message( desktop, &raw_msg ); @@ -2251,6 +2263,7 @@ static void queue_pointer_message( struct pointer *pointer, int repeated ) }; struct hw_msg_source source = { IMDT_UNAVAILABLE, IMDT_TOUCH }; struct desktop *desktop = pointer->desktop; + const desktop_shm_t *desktop_shm = get_shared_desktop( desktop->session_index ); const hw_input_t *input = &pointer->input; unsigned int i, wparam = input->hw.wparam; timeout_t time = get_tick_count(); @@ -2273,8 +2286,8 @@ static void queue_pointer_message( struct pointer *pointer, int repeated ) msg->msg = messages[input->hw.msg - WM_POINTERUPDATE][i]; msg->wparam = wparam; msg->lparam = MAKELONG(x, y); - msg->x = desktop->cursor.x; - msg->y = desktop->cursor.y; + msg->x = desktop_shm->cursor.x; + msg->y = desktop_shm->cursor.y; queue_hardware_message( desktop, msg, 1 ); } @@ -2329,6 +2342,7 @@ static struct pointer *find_pointer_from_id( struct desktop *desktop, unsigned i static void queue_custom_hardware_message( struct desktop *desktop, user_handle_t win, unsigned int origin, const hw_input_t *input ) { + const desktop_shm_t *desktop_shm = get_shared_desktop( desktop->session_index ); struct hw_msg_source source = { IMDT_UNAVAILABLE, origin }; struct thread *foreground; struct pointer *pointer; @@ -2373,8 +2387,8 @@ static void queue_custom_hardware_message( struct desktop *desktop, user_handle_ msg->msg = input->hw.msg; msg->wparam = input->hw.wparam; msg->lparam = input->hw.lparam; - msg->x = desktop->cursor.x; - msg->y = desktop->cursor.y; + msg->x = desktop_shm->cursor.x; + msg->y = desktop_shm->cursor.y; queue_hardware_message( desktop, msg, 1 ); } @@ -2922,6 +2936,7 @@ DECL_HANDLER(send_hardware_message) struct desktop *desktop; unsigned int origin = (req->flags & SEND_HWMSG_INJECTED ? IMO_INJECTED : IMO_HARDWARE); struct msg_queue *sender = req->flags & SEND_HWMSG_INJECTED ? get_current_queue() : NULL; + const desktop_shm_t *desktop_shm; int wait = 0; if (!(desktop = get_hardware_input_desktop( req->win ))) return; @@ -2932,9 +2947,10 @@ DECL_HANDLER(send_hardware_message) set_error( STATUS_ACCESS_DENIED ); return; } + desktop_shm = get_shared_desktop( desktop->session_index ); - reply->prev_x = desktop->cursor.x; - reply->prev_y = desktop->cursor.y; + reply->prev_x = desktop_shm->cursor.x; + reply->prev_y = desktop_shm->cursor.y; switch (req->input.type) { @@ -2952,8 +2968,8 @@ DECL_HANDLER(send_hardware_message) } reply->wait = sender ? wait : 0; - reply->new_x = desktop->cursor.x; - reply->new_y = desktop->cursor.y; + reply->new_x = desktop_shm->cursor.x; + reply->new_y = desktop_shm->cursor.y; release_object( desktop ); } @@ -3646,16 +3662,18 @@ DECL_HANDLER(set_cursor) user_handle_t prev_cursor, new_cursor; struct thread_input *input; struct desktop *desktop; + const desktop_shm_t *desktop_shm; if (!queue) return; input = queue->input; desktop = input->desktop; + desktop_shm = get_shared_desktop( desktop->session_index ); prev_cursor = input->cursor_count < 0 ? 0 : input->cursor; reply->prev_handle = input->cursor; reply->prev_count = input->cursor_count; - reply->prev_x = desktop->cursor.x; - reply->prev_y = desktop->cursor.y; + reply->prev_x = desktop_shm->cursor.x; + reply->prev_y = desktop_shm->cursor.y; if (req->flags & SET_CURSOR_HANDLE) { @@ -3678,8 +3696,8 @@ DECL_HANDLER(set_cursor) new_cursor = input->cursor_count < 0 ? 0 : input->cursor; if (prev_cursor != new_cursor) update_desktop_cursor_handle( desktop, input, new_cursor ); - reply->new_x = desktop->cursor.x; - reply->new_y = desktop->cursor.y; + reply->new_x = desktop_shm->cursor.x; + reply->new_y = desktop_shm->cursor.y; reply->new_clip = desktop->cursor.clip; reply->last_change = desktop->cursor.last_change; } diff --git a/server/user.h b/server/user.h index 6079ab92451..229439fbe5e 100644 --- a/server/user.h +++ b/server/user.h @@ -56,8 +56,6 @@ struct winstation struct global_cursor { - int x; /* cursor position */ - int y; rectangle_t clip; /* cursor clip rectangle */ unsigned int clip_flags; /* last cursor clip flags */ unsigned int last_change; /* time of last position change */ diff --git a/server/winstation.c b/server/winstation.c index 6c47811c6f6..d547e9b8d86 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -272,6 +272,8 @@ static struct desktop *create_desktop( const struct unicode_str *name, unsigned { if (get_error() != STATUS_OBJECT_NAME_EXISTS) { + const desktop_shm_t *desktop_shm; + /* initialize it if it didn't already exist */ desktop->flags = flags; @@ -299,11 +301,18 @@ static struct desktop *create_desktop( const struct unicode_str *name, unsigned list_init( &desktop->pointers ); desktop->session_index = alloc_shared_object(); - if (!get_shared_desktop( desktop->session_index )) + if (!(desktop_shm = get_shared_desktop( desktop->session_index ))) { release_object( desktop ); return NULL; } + + SHARED_WRITE_BEGIN( desktop_shm, desktop_shm_t ) + { + shared->cursor.x = 0; + shared->cursor.y = 0; + } + SHARED_WRITE_END; } else { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
From: Rémi Bernon <rbernon(a)codeweavers.com> Based on a patch by Huw Davies <huw(a)codeweavers.com>. --- server/protocol.def | 1 + server/queue.c | 17 +++++++++++------ server/user.h | 1 - server/winstation.c | 1 + 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/server/protocol.def b/server/protocol.def index d7b854a6c96..5243b091b04 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -882,6 +882,7 @@ struct shared_cursor { int x; /* cursor position */ int y; + unsigned int last_change; /* time of last position change */ }; typedef volatile struct diff --git a/server/queue.c b/server/queue.c index 3780e35d83e..3ebf5e18de1 100644 --- a/server/queue.c +++ b/server/queue.c @@ -468,6 +468,7 @@ static int update_desktop_cursor_pos( struct desktop *desktop, user_handle_t win { const desktop_shm_t *desktop_shm = get_shared_desktop( desktop->session_index ); int updated; + unsigned int time = get_tick_count(); x = max( min( x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left ); y = max( min( y, desktop->cursor.clip.bottom - 1 ), desktop->cursor.clip.top ); @@ -477,11 +478,10 @@ static int update_desktop_cursor_pos( struct desktop *desktop, user_handle_t win updated = shared->cursor.x != x || shared->cursor.y != y; shared->cursor.x = x; shared->cursor.y = y; + shared->cursor.last_change = time; } SHARED_WRITE_END; - desktop->cursor.last_change = get_tick_count(); - if (!win || !is_window_visible( win ) || is_window_transparent( win )) win = shallow_window_from_point( desktop, x, y ); if (update_desktop_cursor_window( desktop, win )) updated = 1; @@ -2005,7 +2005,7 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons struct rawinput_message raw_msg; struct message *msg; struct thread *foreground; - unsigned int i, time, flags; + unsigned int i, time = get_tick_count(), flags; struct hw_msg_source source = { IMDT_MOUSE, origin }; int wait = 0, x, y; @@ -2026,10 +2026,15 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons WM_MOUSEHWHEEL /* 0x1000 = MOUSEEVENTF_HWHEEL */ }; - desktop->cursor.last_change = get_tick_count(); + SHARED_WRITE_BEGIN( desktop_shm, desktop_shm_t ) + { + shared->cursor.last_change = time; + } + SHARED_WRITE_END; + flags = input->mouse.flags; time = input->mouse.time; - if (!time) time = desktop->cursor.last_change; + if (!time) time = desktop_shm->cursor.last_change; if (flags & MOUSEEVENTF_MOVE) { @@ -3699,7 +3704,7 @@ DECL_HANDLER(set_cursor) reply->new_x = desktop_shm->cursor.x; reply->new_y = desktop_shm->cursor.y; reply->new_clip = desktop->cursor.clip; - reply->last_change = desktop->cursor.last_change; + reply->last_change = desktop_shm->cursor.last_change; } /* Get the history of the 64 last cursor positions */ diff --git a/server/user.h b/server/user.h index 229439fbe5e..3adfe29073b 100644 --- a/server/user.h +++ b/server/user.h @@ -58,7 +58,6 @@ struct global_cursor { rectangle_t clip; /* cursor clip rectangle */ unsigned int clip_flags; /* last cursor clip flags */ - unsigned int last_change; /* time of last position change */ user_handle_t win; /* window that contains the cursor */ }; diff --git a/server/winstation.c b/server/winstation.c index d547e9b8d86..fcfcb8f419e 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -311,6 +311,7 @@ static struct desktop *create_desktop( const struct unicode_str *name, unsigned { shared->cursor.x = 0; shared->cursor.y = 0; + shared->cursor.last_change = 0; } SHARED_WRITE_END; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
From: Rémi Bernon <rbernon(a)codeweavers.com> Based on a patch by Huw Davies <huw(a)codeweavers.com>. --- dlls/win32u/input.c | 18 +++++++++--------- dlls/win32u/winstation.c | 7 ------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index 04532e7d015..2b0f59095e2 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -739,22 +739,22 @@ BOOL WINAPI NtUserSetCursorPos( INT x, INT y ) */ BOOL get_cursor_pos( POINT *pt ) { + struct object_lock lock = OBJECT_LOCK_INIT; + const desktop_shm_t *desktop_shm; BOOL ret; - DWORD last_change; + DWORD last_change = 0; + NTSTATUS status; UINT dpi; if (!pt) return FALSE; - SERVER_START_REQ( set_cursor ) + while ((status = get_shared_desktop( &lock, &desktop_shm )) == STATUS_PENDING) { - if ((ret = !wine_server_call( req ))) - { - pt->x = reply->new_x; - pt->y = reply->new_y; - last_change = reply->last_change; - } + pt->x = desktop_shm->cursor.x; + pt->y = desktop_shm->cursor.y; + last_change = desktop_shm->cursor.last_change; } - SERVER_END_REQ; + ret = !status; /* query new position from graphics driver if we haven't updated recently */ if (ret && NtGetTickCount() - last_change > 100) ret = user_driver->pGetCursorPos( pt ); diff --git a/dlls/win32u/winstation.c b/dlls/win32u/winstation.c index 5abbdd6e067..260587c05d8 100644 --- a/dlls/win32u/winstation.c +++ b/dlls/win32u/winstation.c @@ -487,17 +487,10 @@ BOOL WINAPI NtUserSetThreadDesktop( HDESK handle ) struct user_thread_info *thread_info = get_user_thread_info(); struct user_key_state_info *key_state_info = thread_info->key_state; obj_locator_t *locator = &get_session_thread_data()->shared_desktop; - struct object_lock lock = OBJECT_LOCK_INIT; - const desktop_shm_t *desktop_shm; - thread_info->client_info.top_window = 0; thread_info->client_info.msg_window = 0; if (key_state_info) key_state_info->time = 0; memset( locator, 0, sizeof(*locator) ); - - while (get_shared_desktop( &lock, &desktop_shm ) == STATUS_PENDING) - /* nothing */; - if (was_virtual_desktop != is_virtual_desktop()) update_display_cache( TRUE ); } return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=145252 Your paranoid android. === debian11 (build log) === error: patch failed: tools/make_requests:53 error: patch failed: dlls/win32u/sysparams.c:6210 Task: Patch failed to apply === debian11b (build log) === error: patch failed: tools/make_requests:53 error: patch failed: dlls/win32u/sysparams.c:6210 Task: Patch failed to apply
On Mon Apr 29 13:16:19 2024 +0000, Jinoh Kang wrote:
(nit) Avoid directly manipulating ref if possible (ref is zero-initialized by calloc) ```suggestion:-1+0 shared_session = shared_session_acquire( session ); ``` Initializing ref count to 1 is a common pattern in Wine. I moved it next to the calloc to make it more like what we often do.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69014
On Mon Apr 29 13:15:42 2024 +0000, Jinoh Kang wrote:
Empty unions either need a `char dummy;` or should be squashed into future commits in sequence I think. MSVC complains:
error C2016: C requires that a struct or union has at least one member Fixed, although I don't think we support building wineserver with MSVC.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69015
On Mon Apr 29 13:16:00 2024 +0000, Jinoh Kang wrote:
(nit) I think it's a common practice not to repeat macro arguments to avoid surprising behavior due to double execution of code; is there any reason you decided against it? ```suggestion:-2+0 const type *__shared = (object_shm); \ type *shared = (type *)__shared; \ session_object_t *__obj = CONTAINING_RECORD( shared, session_object_t, shm ); \ ``` Changed. There was no good reason, only cosmetics.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69016
On Mon Apr 29 13:16:27 2024 +0000, Jinoh Kang wrote:
This include is a duplicate. ```suggestion:-1+0 ``` Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69017
On Mon Apr 29 13:17:46 2024 +0000, Jinoh Kang wrote:
(nit) consistency ```suggestion:-0+0 session.object_capacity = mapping->size / sizeof(*session.objects); ``` Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69018
On Mon Apr 29 13:18:01 2024 +0000, Jinoh Kang wrote:
(nit) consistency ```suggestion:-0+0 capacity = size / sizeof(*session.objects); ``` Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69019
On Mon Apr 29 13:18:23 2024 +0000, Jinoh Kang wrote:
(nit) consistency ```suggestion:-0+0 session->object_capacity = size / sizeof(*session->objects); ``` Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69020
On Mon Apr 29 13:18:46 2024 +0000, Jinoh Kang wrote:
(nit) consistency ```suggestion:-0+0 size = sizeof(*session.objects) * capacity; ``` Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69021
On Mon Apr 29 13:15:42 2024 +0000, Rémi Bernon wrote:
Fixed, although I don't think we support building wineserver with MSVC. Thanks. include/wine/server_protocol.h is shared with PE side, so I was taking extra caution. Not sure if we have MSVC build working in general, though.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69084
On Sun Apr 28 09:14:30 2024 +0000, Rémi Bernon wrote:
changed this line in [version 32 of the diff](/wine/wine/-/merge_requests/3103/diffs?diff_id=111587&start_sha=9e3075b55b5d9b1f4f122fb407ecbf520113ef02#35a0b3c4868b938764504464c7e3891ad9581e77_161_161) Much better, thanks.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69085
(commits) lone newline diff hunk that perhaps should be either deleted or squashed into "win32u: Open the global session shared mapping." Note that the newline itself is fine, since we do have precedent\* of separating C std includes and `#include <pthread.h>`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69094
This merge request was approved by Jinoh Kang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103
Notes on alternative designs: 1. The refcount mechanism for the session object looks a little superflous at the first glance. But the alternative would be to lock the object for the entirety of the operation, which seems about as complex (we need to make sure not to forget to lock/unlock, just like we do acquire/release now). Additionally, a prolonged critical section tend to introduce unnecessary contention and potential for deadlock. 2. Note that we store the index, instead of a pointer to the shared memory object in the desktop. I think this design makes it clear that a desktop merely references but doesn't own the portion of the shared memory. We can of course store the `desktop_shm_t` pointer and eliminate `get_shared_desktop()`, but then we'd need to do pointer arithmetic to get the index back. My experience tells me that it's quite easy to mess up the arithmetic and choose the wrong base address, especially during refactoring. (By the way, `SHARED_WRITE_BEGIN` is necessary even if we held a direct pointer. Also `unsigned int index` is 4 bytes less than `desktop_shm_t *` on 64-bit, if it's of any significance.) 3. The object lock instead of `SHARED_READ_BEGIN`. We decided that it's more trouble than any help; this MR has a whole thread that argues against it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_69095
participants (3)
-
Jinoh Kang (＠iamahuman) -
Marvin -
Rémi Bernon