[PATCH v20 0/8] 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.
-- v20: 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: 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.
From: Rémi Bernon rbernon@codeweavers.com
--- server/directory.c | 3 +++ server/file.h | 17 +++++++++++++++++ server/mapping.c | 37 +++++++++++++++++++++++++++++++++++++ server/protocol.def | 23 +++++++++++++++++++++++ 4 files changed, 80 insertions(+)
diff --git a/server/directory.c b/server/directory.c index 23d7eb0a2b7..20a0d1d49e8 100644 --- a/server/directory.c +++ b/server/directory.c @@ -439,8 +439,10 @@ 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; @@ -489,6 +491,7 @@ void init_directories( struct fd *intl_fd ) /* mappings */ release_object( create_fd_mapping( &dir_nls->obj, &intl_str, intl_fd, OBJ_PERMANENT, NULL )); release_object( create_user_data_mapping( &dir_kernel->obj, &user_data_str, OBJ_PERMANENT, NULL )); + release_object( create_session_mapping( &dir_kernel->obj, &session_str, OBJ_PERMANENT, NULL )); release_object( intl_fd );
release_object( named_pipe_device ); diff --git a/server/file.h b/server/file.h index 7f2d1637863..85aed168966 100644 --- a/server/file.h +++ b/server/file.h @@ -188,6 +188,23 @@ 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 object *create_session_mapping( struct object *root, const struct unicode_str *name, + unsigned int attr, const struct security_descriptor *sd ); + +#define SHARED_WRITE_BEGIN( object, type ) \ + do { \ + const type *__shared = (object); \ + type *shared = (type *)__shared; \ + LONG64 __seq = shared->obj.seq + 1, __end = __seq + 1; \ + assert( (__seq & 1) != 0 ); \ + __WINE_ATOMIC_STORE_RELEASE( &shared->obj.seq, &__seq ); \ + do + +#define SHARED_WRITE_END \ + while(0); \ + assert( __seq == shared->obj.seq ); \ + __WINE_ATOMIC_STORE_RELEASE( &shared->obj.seq, &__end ); \ + } while(0)
/* device functions */
diff --git a/server/mapping.c b/server/mapping.c index 6b0de1b8b94..972b0ffb073 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -225,6 +225,15 @@ static const mem_size_t granularity_mask = 0xffff; static struct addr_range ranges32; static struct addr_range ranges64;
+struct session +{ + const session_shm_t *shared; + unsigned int object_count; + 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 +1265,34 @@ int get_page_size(void) return page_mask + 1; }
+struct object *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; + struct mapping *mapping; + void *tmp; + + if (!(mapping = create_mapping( root, name, attr, 0x10000, SEC_COMMIT, 0, access, sd ))) return NULL; + if ((tmp = mmap( NULL, mapping->size, PROT_READ | PROT_WRITE, MAP_SHARED, get_unix_fd( mapping->fd ), 0 )) == MAP_FAILED) + { + release_object( &mapping->obj ); + return NULL; + } + + session_mapping = mapping; + session.object_capacity = (mapping->size - offsetof(session_shm_t, objects[0])) / sizeof(session_obj_t); + session.shared = tmp; + + SHARED_WRITE_BEGIN( session.shared, session_shm_t ) + { + shared->obj.id++; + shared->size = mapping->size; + } + SHARED_WRITE_END; + + return &mapping->obj; +} + 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 8b51618ebe0..a92065f6eb4 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -879,6 +879,29 @@ typedef struct lparam_t info; } cursor_pos_t;
+/****************************************************************/ +/* shared session mapping structures */ + +typedef volatile struct +{ + LONG64 seq; /* sequence number - server updating if (seq & 1) != 0 */ + unsigned __int64 id; /* object unique id, object data is valid if != 0 */ +} object_shm_t; + +typedef volatile union +{ + object_shm_t obj; +} session_obj_t; + +typedef volatile struct +{ + object_shm_t obj; + mem_size_t size; /* session shared mapping size */ + session_obj_t objects[]; +} session_shm_t; + +C_ASSERT(sizeof(session_shm_t) == offsetof(session_shm_t, objects[0])); + /****************************************************************/ /* Request declarations */
From: Rémi Bernon rbernon@codeweavers.com
--- include/winnt.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/include/winnt.h b/include/winnt.h index 0e0d1cb5170..24df62404c5 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -6990,11 +6990,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 */
@@ -7028,6 +7031,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(); @@ -7214,6 +7223,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 );
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/win32u/win32u_private.h | 23 ++++++++ dlls/win32u/winstation.c | 109 ++++++++++++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 1 deletion(-)
diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index d5f010a8249..f7d25ffc6f3 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -362,4 +362,27 @@ static inline BOOL intersect_rect( RECT *dst, const RECT *src1, const RECT *src2 return !IsRectEmpty( dst ); }
+#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 + +#define SHARED_READ_BEGIN( ptr, type ) \ + do { \ + const type *shared = (ptr); \ + LONG64 __seq; \ + do { \ + while ((__seq = ReadNoFence64( &shared->obj.seq )) & 1) \ + YieldProcessor(); \ + __SHARED_READ_FENCE; \ + do + +#define SHARED_READ_END \ + while (0); \ + __SHARED_READ_FENCE; \ + } while (ReadNoFence64( &shared->obj.seq ) != __seq); \ + } while(0) + #endif /* __WINE_WIN32U_PRIVATE */ diff --git a/dlls/win32u/winstation.c b/dlls/win32u/winstation.c index b187b246941..7f3d2257e64 100644 --- a/dlls/win32u/winstation.c +++ b/dlls/win32u/winstation.c @@ -22,9 +22,12 @@ #pragma makedep unix #endif
+#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 +43,108 @@ WINE_DECLARE_DEBUG_CHANNEL(win);
#define DESKTOP_ALL_ACCESS 0x01ff
+static pthread_mutex_t session_lock = PTHREAD_MUTEX_INITIALIZER; +static struct shared_session *shared_session; + +struct shared_session +{ + LONG ref; + UINT64 id; + SIZE_T size; + const session_shm_t *shared; +}; + +static struct shared_session *shared_session_acquire( struct shared_session *session ) +{ + InterlockedIncrement( &session->ref ); + return session; +} + +static void shared_session_release( struct shared_session *session ) +{ + if (!InterlockedDecrement( &session->ref )) + { + NtUnmapViewOfSection( GetCurrentProcess(), (void *)session->shared ); + free( session ); + } +} + +static NTSTATUS map_shared_session_section( struct shared_session *session, HANDLE handle ) +{ + NTSTATUS status; + + while (!(status = NtMapViewOfSection( handle, GetCurrentProcess(), (void **)&session->shared, 0, 0, + NULL, &session->size, ViewUnmap, 0, PAGE_READONLY ))) + { + BOOL invalid; + + SHARED_READ_BEGIN( session->shared, session_shm_t ) + { + if ((invalid = session->size != shared->size)) break; + session->id = shared->obj.id; + } + SHARED_READ_END; + if (!invalid) break; + + NtUnmapViewOfSection( GetCurrentProcess(), (void *)session->shared ); + session->shared = NULL; + session->size = 0; + } + + return status; +} + +static struct shared_session *get_shared_session( BOOL force ) +{ + struct shared_session *session; + + pthread_mutex_lock( &session_lock ); + + if (!shared_session || force) + { + 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; + HANDLE handle; + + if (!(session = calloc( 1, sizeof(*session) ))) + { + pthread_mutex_unlock( &session_lock ); + return NULL; + } + session->ref = 1; + + InitializeObjectAttributes( &attr, &name, 0, NULL, NULL ); + if (!(status = NtOpenSection( &handle, SECTION_MAP_READ | SECTION_QUERY, &attr ))) + { + status = map_shared_session_section( session, handle ); + NtClose( handle ); + } + + if (status) + { + ERR( "Failed to map session mapping, status %#x\n", status ); + free( session ); + } + else + { + if (shared_session) shared_session_release( shared_session ); + shared_session = session; + } + } + + session = shared_session_acquire( shared_session ); + + pthread_mutex_unlock( &session_lock ); + + return session; +} + BOOL is_virtual_desktop(void) { HANDLE desktop = NtUserGetThreadDesktop( GetCurrentThreadId() ); @@ -622,6 +727,8 @@ void winstation_init(void)
static const WCHAR winsta0[] = {'W','i','n','S','t','a','0',0};
+ shared_session = get_shared_session( FALSE ); + if (params->Desktop.Length) { buffer = malloc( params->Desktop.Length + sizeof(WCHAR) );
From: Rémi Bernon rbernon@codeweavers.com
--- server/file.h | 4 +++ server/mapping.c | 69 +++++++++++++++++++++++++++++++++++++++++++++ server/protocol.def | 7 +++++ server/user.h | 1 + server/winstation.c | 17 +++++++++++ 5 files changed, 98 insertions(+)
diff --git a/server/file.h b/server/file.h index 85aed168966..51fc6eb1858 100644 --- a/server/file.h +++ b/server/file.h @@ -191,6 +191,10 @@ extern struct object *create_user_data_mapping( struct object *root, const struc extern struct object *create_session_mapping( struct object *root, const struct unicode_str *name, unsigned int attr, const struct security_descriptor *sd );
+extern int alloc_shared_object(void); +extern void free_shared_object( int index ); +extern const desktop_shm_t *get_shared_desktop( int index ); + #define SHARED_WRITE_BEGIN( object, type ) \ do { \ const type *__shared = (object); \ diff --git a/server/mapping.c b/server/mapping.c index 972b0ffb073..f1414508114 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -230,6 +230,7 @@ struct session const session_shm_t *shared; unsigned int object_count; unsigned int object_capacity; + unsigned __int64 next_object_id; }; static struct mapping *session_mapping; static struct session session; @@ -1293,6 +1294,74 @@ struct object *create_session_mapping( struct object *root, const struct unicode return &mapping->obj; }
+static int grow_session_mapping(void) +{ + unsigned int capacity; + mem_size_t size; + int unix_fd; + void *tmp; + + capacity = session.object_capacity * 3 / 2; + size = offsetof(session_shm_t, objects[capacity]); + size = (size + page_mask) & ~((mem_size_t)page_mask); + + unix_fd = get_unix_fd( session_mapping->fd ); + if (!grow_file( unix_fd, size )) return -1; + session_mapping->size = size; + + if ((tmp = mmap( NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, unix_fd, 0 )) == MAP_FAILED) return -1; + munmap( (void *)session.shared, session.shared->size ); + session.shared = tmp; + + SHARED_WRITE_BEGIN( session.shared, session_shm_t ) + { + shared->obj.id++; + shared->size = size; + } + SHARED_WRITE_END; + + return 0; +} + +int alloc_shared_object(void) +{ + int index; + + for (index = 0; index < session.object_count; index++) + if (!session.shared->objects[index].obj.id) + break; + if (index == session.object_count) + { + if (session.object_count == session.object_capacity && grow_session_mapping()) return -1; + index = session.object_count++; + } + + SHARED_WRITE_BEGIN( &session.shared->objects[index], session_obj_t ) + { + shared->obj.id = ++session.next_object_id; + } + SHARED_WRITE_END; + + return index; +} + +void free_shared_object( int index ) +{ + if (index < 0) return; + + SHARED_WRITE_BEGIN( &session.shared->objects[index], session_obj_t ) + { + shared->obj.id = 0; + } + SHARED_WRITE_END; +} + +const desktop_shm_t *get_shared_desktop( int index ) +{ + if (index < 0) return NULL; + return &session.shared->objects[index].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 a92065f6eb4..1a2b07f79d8 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -888,9 +888,15 @@ typedef volatile struct unsigned __int64 id; /* object unique id, object data is valid if != 0 */ } object_shm_t;
+typedef volatile struct +{ + object_shm_t obj; +} desktop_shm_t; + typedef volatile union { object_shm_t obj; + desktop_shm_t desktop; } session_obj_t;
typedef volatile struct @@ -2803,6 +2809,7 @@ enum coords_relative thread_id_t tid; /* thread id */ @REPLY obj_handle_t handle; /* handle to the desktop */ + int index; /* index of desktop object in session shared memory */ @END
diff --git a/server/user.h b/server/user.h index 8fa55e09b0f..32f648b4dc2 100644 --- a/server/user.h +++ b/server/user.h @@ -76,6 +76,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 */ + int session_index; /* desktop index in session shared memory */ };
/* user handles functions */ diff --git a/server/winstation.c b/server/winstation.c index 5903497d61e..6b708ad782f 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -240,6 +240,13 @@ static struct desktop *create_desktop( const struct unicode_str *name, unsigned memset( desktop->keystate, 0, sizeof(desktop->keystate) ); list_add_tail( &winstation->desktops, &desktop->entry ); list_init( &desktop->hotkeys ); + desktop->session_index = -1; + + if ((desktop->session_index = alloc_shared_object()) == -1) + { + release_object( desktop ); + return NULL; + } } else { @@ -298,6 +305,7 @@ static void desktop_destroy( struct object *obj ) if (desktop->close_timeout) remove_timeout_user( desktop->close_timeout ); list_remove( &desktop->entry ); release_object( desktop->winstation ); + free_shared_object( desktop->session_index ); }
/* retrieve the thread desktop, checking the handle access rights */ @@ -601,10 +609,19 @@ DECL_HANDLER(close_desktop) /* get the thread current desktop */ DECL_HANDLER(get_thread_desktop) { + struct desktop *desktop; struct thread *thread;
if (!(thread = get_thread_from_id( req->tid ))) return; reply->handle = thread->desktop; + reply->index = -1; + + if ((desktop = get_thread_desktop( thread, 0 ))) + { + reply->index = desktop->session_index; + release_object( desktop ); + } + release_object( thread ); }
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/win32u/ntuser_private.h | 1 + dlls/win32u/sysparams.c | 1 + dlls/win32u/win32u_private.h | 13 +++++ dlls/win32u/winstation.c | 94 ++++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+)
diff --git a/dlls/win32u/ntuser_private.h b/dlls/win32u/ntuser_private.h index 3b6cab5bdc9..524195941f3 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_object *shared_desktop; /* thread desktop shared session object */ };
C_ASSERT( sizeof(struct user_thread_info) <= sizeof(((TEB *)0)->Win32ClientInfo) ); diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 673082056b1..7b8f4f6f1d1 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -6204,6 +6204,7 @@ static void thread_detach(void) destroy_thread_windows(); cleanup_imm_thread(); NtClose( thread_info->server_queue ); + if (thread_info->shared_desktop) session_object_release( thread_info->shared_desktop );
exiting_thread_id = 0; } diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index f7d25ffc6f3..f106c5c7cd6 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -194,6 +194,19 @@ extern void user_unlock(void); extern void user_check_not_lock(void);
/* winstation.c */ + +struct shared_session; +struct session_object +{ + LONG ref; + UINT64 id; + const session_obj_t *shared; + struct shared_session *session; +}; + +extern void session_object_release( struct session_object *desktop ); +extern struct session_object *get_shared_desktop( BOOL force ); + extern BOOL is_virtual_desktop(void);
/* window.c */ diff --git a/dlls/win32u/winstation.c b/dlls/win32u/winstation.c index 7f3d2257e64..d4255d07bb0 100644 --- a/dlls/win32u/winstation.c +++ b/dlls/win32u/winstation.c @@ -145,6 +145,97 @@ static struct shared_session *get_shared_session( BOOL force ) return session; }
+static struct session_object *session_object_acquire( struct session_object *desktop ) +{ + InterlockedIncrement( &desktop->ref ); + return desktop; +} + +void session_object_release( struct session_object *desktop ) +{ + if (!InterlockedDecrement( &desktop->ref )) + { + shared_session_release( desktop->session ); + free( desktop ); + } +} + +enum object_type +{ + OBJECT_TYPE_DESKTOP = 1, +}; + +static int get_thread_session_object_index( UINT tid, enum object_type type ) +{ + switch (type) + { + case OBJECT_TYPE_DESKTOP: + SERVER_START_REQ( get_thread_desktop ) + { + req->tid = tid; + if (wine_server_call_err( req )) return -1; + return reply->index; + } + SERVER_END_REQ; + break; + } + + return -1; +} + +static struct session_object *get_thread_session_object( UINT tid, enum object_type type ) +{ + struct shared_session *session; + struct session_object *object; + BOOL invalid = FALSE; + unsigned int idx; + + if (!(object = calloc( 1, sizeof(*object) ))) return NULL; + object->ref = 1; + + while ((session = get_shared_session( invalid ))) + { + if ((idx = get_thread_session_object_index( tid, type )) == -1) break; + + SHARED_READ_BEGIN( session->shared, session_shm_t ) + { + if ((invalid = session->id != shared->obj.id)) break; + object->shared = &shared->objects[idx]; + object->id = object->shared->obj.id; + } + SHARED_READ_END; + + if (!invalid) break; + shared_session_release( session ); + } + + if (!object->shared) + { + WARN( "Failed to find object type %u for thread %u\n", type, tid ); + if (session) shared_session_release( session ); + free( object ); + return NULL; + } + + object->session = session; + return object; +} + +struct session_object *get_shared_desktop( BOOL force ) +{ + struct user_thread_info *thread_info = get_user_thread_info(); + struct session_object *desktop; + + if (!thread_info->shared_desktop || force) + { + if (!(desktop = get_thread_session_object( GetCurrentThreadId(), OBJECT_TYPE_DESKTOP ))) return NULL; + if (thread_info->shared_desktop) session_object_release( thread_info->shared_desktop ); + thread_info->shared_desktop = desktop; + } + + return session_object_acquire( thread_info->shared_desktop ); +} + BOOL is_virtual_desktop(void) { HANDLE desktop = NtUserGetThreadDesktop( GetCurrentThreadId() ); @@ -369,6 +460,9 @@ BOOL WINAPI NtUserSetThreadDesktop( HDESK handle ) thread_info->client_info.top_window = 0; thread_info->client_info.msg_window = 0; if (key_state_info) key_state_info->time = 0; + if (thread_info->shared_desktop) session_object_release( thread_info->shared_desktop ); + thread_info->shared_desktop = NULL; + get_shared_desktop( FALSE ); if (was_virtual_desktop != is_virtual_desktop()) update_display_cache( TRUE ); } return ret;
From: Rémi Bernon rbernon@codeweavers.com
Based on a patch by Huw Davies huw@codeweavers.com. --- server/protocol.def | 7 +++++ server/queue.c | 73 ++++++++++++++++++++++++++++----------------- server/user.h | 2 -- 3 files changed, 52 insertions(+), 30 deletions(-)
diff --git a/server/protocol.def b/server/protocol.def index 1a2b07f79d8..c85d0a72b10 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -882,6 +882,12 @@ typedef struct /****************************************************************/ /* shared session mapping structures */
+struct shared_cursor +{ + int x; /* cursor position */ + int y; +}; + typedef volatile struct { LONG64 seq; /* sequence number - server updating if (seq & 1) != 0 */ @@ -891,6 +897,7 @@ typedef volatile struct typedef volatile struct { object_shm_t obj; + struct shared_cursor cursor; /* global cursor information */ } desktop_shm_t;
typedef volatile union diff --git a/server/queue.c b/server/queue.c index 6f38227aa84..58396dfa912 100644 --- a/server/queue.c +++ b/server/queue.c @@ -416,6 +416,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;
@@ -424,8 +425,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 ); } @@ -463,13 +464,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 )) @@ -515,15 +523,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; int x, y;
@@ -541,9 +551,9 @@ void set_clip_rectangle( struct desktop *desktop, const rectangle_t *rect, unsig else desktop->cursor.clip = top_rect;
/* 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 ); @@ -1664,6 +1674,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; @@ -1696,8 +1707,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 ))) { @@ -1991,6 +2002,7 @@ done: 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; @@ -2029,19 +2041,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 ))) @@ -2053,7 +2065,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 );
enum_processes( queue_rawinput_message, &raw_msg ); @@ -2233,6 +2245,7 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c 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 rawinput_message raw_msg; struct message *msg; @@ -2263,8 +2276,8 @@ static void queue_custom_hardware_message( struct desktop *desktop, user_handle_ msg->msg = input->hw.msg; msg->wparam = 0; 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 ); } @@ -2771,8 +2784,10 @@ DECL_HANDLER(send_hardware_message) struct desktop *desktop; unsigned int origin = (req->flags & SEND_HWMSG_INJECTED ? IMO_INJECTED : IMO_HARDWARE); struct msg_queue *sender = get_current_queue(); + const desktop_shm_t *desktop_shm;
if (!(desktop = get_thread_desktop( current, 0 ))) return; + desktop_shm = get_shared_desktop( desktop->session_index );
if (req->win) { @@ -2785,8 +2800,8 @@ DECL_HANDLER(send_hardware_message) } }
- 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) { @@ -2804,8 +2819,8 @@ DECL_HANDLER(send_hardware_message) } if (thread) release_object( thread );
- 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 ); }
@@ -3490,15 +3505,17 @@ DECL_HANDLER(set_cursor) struct msg_queue *queue = get_current_queue(); 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 );
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) { @@ -3521,8 +3538,8 @@ DECL_HANDLER(set_cursor) if (req->flags & (SET_CURSOR_HANDLE | SET_CURSOR_COUNT)) update_desktop_cursor_handle( desktop, input );
- 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 32f648b4dc2..8ff3c3bfc33 100644 --- a/server/user.h +++ b/server/user.h @@ -54,8 +54,6 @@ struct winstation
struct global_cursor { - int x; /* cursor position */ - int y; rectangle_t clip; /* cursor clip rectangle */ unsigned int last_change; /* time of last position change */ user_handle_t win; /* window that contains the cursor */
From: Rémi Bernon rbernon@codeweavers.com
Based on a patch by Huw Davies huw@codeweavers.com. --- server/protocol.def | 1 + server/queue.c | 17 +++++++++++------ server/user.h | 1 - 3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/server/protocol.def b/server/protocol.def index c85d0a72b10..de3704f520a 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -886,6 +886,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 58396dfa912..f39c7e6cfcb 100644 --- a/server/queue.c +++ b/server/queue.c @@ -466,6 +466,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 ); @@ -475,11 +476,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; @@ -2008,7 +2008,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;
@@ -2029,10 +2029,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) { @@ -3541,7 +3546,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 8ff3c3bfc33..6b93f9f4ca8 100644 --- a/server/user.h +++ b/server/user.h @@ -55,7 +55,6 @@ struct winstation struct global_cursor { rectangle_t clip; /* cursor clip rectangle */ - unsigned int last_change; /* time of last position change */ user_handle_t win; /* window that contains the cursor */ };
From: Rémi Bernon rbernon@codeweavers.com
Based on a patch by Huw Davies huw@codeweavers.com. --- dlls/win32u/input.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index ef8d564c264..eb2b7b92bdb 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -749,22 +749,27 @@ BOOL WINAPI NtUserSetCursorPos( INT x, INT y ) */ BOOL get_cursor_pos( POINT *pt ) { - BOOL ret; - DWORD last_change; + BOOL invalid = FALSE, ret = FALSE; + struct session_object *object; + DWORD last_change = 0; UINT dpi;
if (!pt) return FALSE;
- SERVER_START_REQ( set_cursor ) + while ((object = get_shared_desktop( invalid ))) { - if ((ret = !wine_server_call( req ))) + SHARED_READ_BEGIN( &object->shared->desktop, desktop_shm_t ) { - pt->x = reply->new_x; - pt->y = reply->new_y; - last_change = reply->last_change; + if ((invalid = object->id != shared->obj.id)) break; + pt->x = shared->cursor.x; + pt->y = shared->cursor.y; + last_change = shared->cursor.last_change; + ret = TRUE; } + SHARED_READ_END; + session_object_release( object ); + if (!invalid) break; } - SERVER_END_REQ;
/* query new position from graphics driver if we haven't updated recently */ if (ret && NtGetTickCount() - last_change > 100) ret = user_driver->pGetCursorPos( pt );
On Tue Feb 20 16:58:56 2024 +0000, Rémi Bernon wrote:
changed this line in [version 20 of the diff](/wine/wine/-/merge_requests/3103/diffs?diff_id=100722&start_sha=b4491ba1cbd02d0f365a22bee2f2f2b97631725b#35a0b3c4868b938764504464c7e3891ad9581e77_101_125)
Yeah actually I think I can just use `NtMapViewOfSection` with size = 0. Did that instead and split it into a separate helper.
On Tue Feb 20 16:58:57 2024 +0000, Rémi Bernon wrote:
changed this line in [version 20 of the diff](/wine/wine/-/merge_requests/3103/diffs?diff_id=100722&start_sha=b4491ba1cbd02d0f365a22bee2f2f2b97631725b#2b37d5dc1c3bb6a637f043480c038caea64531bb_1352_1323)
Right. I'm now growing the file instead, using object ids like we discussed.
On Tue Feb 20 13:08:45 2024 +0000, Jinoh Kang wrote:
To avoid unnecesary growth, I suggest that you keep track of number of allocated (`!invalid && !destroyed`) objects and use it to calculate capacity. Otherwise, we end up unnecessarily extending size when e.g., 50% of the mapping is actually unused (invalid).
Indeed. I've changed this to better recycle objects. Growing only happens now when we're out of objects.
On Tue Feb 20 16:58:59 2024 +0000, Rémi Bernon wrote:
changed this line in [version 20 of the diff](/wine/wine/-/merge_requests/3103/diffs?diff_id=100722&start_sha=b4491ba1cbd02d0f365a22bee2f2f2b97631725b#2b37d5dc1c3bb6a637f043480c038caea64531bb_1314_1308)
That's still true at least for the session mapping initialization but I think it's better to not create a precedent by casting the shared memory pointer to be writable.
On Tue Feb 20 16:58:54 2024 +0000, Rémi Bernon wrote:
changed this line in [version 20 of the diff](/wine/wine/-/merge_requests/3103/diffs?diff_id=100722&start_sha=b4491ba1cbd02d0f365a22bee2f2f2b97631725b#35a0b3c4868b938764504464c7e3891ad9581e77_146_168)
I know we don't consistently do these checks and that they are not really useful, but I prefer to (either that or assert, I don't mind but I believe we rarely do assert).
Anyway I think it's addressed now. I've also simplified the code by handling all object types in a unique `struct session_object` structure. Makes it necessary to access the shared data through the session object union, which may be more sensitive to typos, but it's less code.
v20: Better recycle objects, grow the mapping file instead of replacing it.
Jinoh Kang (@iamahuman) commented about server/mapping.c:
- return 0;
+}
+int alloc_shared_object(void) +{
- int index;
- for (index = 0; index < session.object_count; index++)
if (!session.shared->objects[index].obj.id)break;- if (index == session.object_count)
- {
if (session.object_count == session.object_capacity && grow_session_mapping()) return -1;index = session.object_count++;- }
Not a blocker, but wouldn't it be faster if we didn't scan empty slots before we actually run out of capacity?
Jinoh Kang (@iamahuman) commented about dlls/win32u/winstation.c:
- }
+}
+static NTSTATUS map_shared_session_section( struct shared_session *session, HANDLE handle ) +{
- NTSTATUS status;
- while (!(status = NtMapViewOfSection( handle, GetCurrentProcess(), (void **)&session->shared, 0, 0,
NULL, &session->size, ViewUnmap, 0, PAGE_READONLY )))- {
BOOL invalid;SHARED_READ_BEGIN( session->shared, session_shm_t ){if ((invalid = session->size != shared->size)) break;session->id = shared->obj.id;
`break`-ing out of macro-internal do-while(0) is confusing and unprecedented[^1][^2]. If I see `break`, my eyes go to the nearest ancestor while/for/do/switch, not some odd-looking Wine-specific macro that I don't necessarily have intimate knowledge of.
I don't think people[^3] are supposed to *have* to understand that `SHARED_READ_BEGIN` is a do-while loop in disguise, just to make sense of this control flow.
```suggestion:-1+0 if (!(invalid = session->size != shared->size)) session->id = shared->obj.id; ```
```suggestion:-1+0 if ((invalid = session->size != shared->size)) goto invalid; session->id = shared->obj.id; ```
```suggestion:-1+0 if ((invalid = session->size != shared->size)) SHARED_READ_GOTO_BLOCK_EXIT(); // Custom macro session->id = shared->obj.id; ```
[^1]: Excluding perhaps `break` directly *inside* a switch, which still doesn't exit the implicit do-while loop, or `return`, which doesn't change its jump target whether the macro-internal do-while loop exists around it or not. [^2]: I could be wrong; please feel free to correct me if this is not the case. [^3]: Who wants to focus on the bug/feature at hand and doesn't necessarily care how `SHARED_READ_BEGIN` works
Jinoh Kang (@iamahuman) commented about dlls/win32u/winstation.c:
- }
- object->session = session;
- return object;
+}
+struct session_object *get_shared_desktop( BOOL force ) +{
- struct user_thread_info *thread_info = get_user_thread_info();
- struct session_object *desktop;
- if (!thread_info->shared_desktop || force)
- {
if (!(desktop = get_thread_session_object( GetCurrentThreadId(), OBJECT_TYPE_DESKTOP ))) return NULL;if (thread_info->shared_desktop) session_object_release( thread_info->shared_desktop );thread_info->shared_desktop = desktop;
(Tentative review, feel free to skip for now if you don't think this is applicable.)
Once our cached desktop go stale (due to e.g., mapping growth), the old cache won't be freed until the next `get_shared_desktop` call or thread exit.
While the old stale cache is alive, it will hold a reference to the `shared_section`, which will coexist with new `shared_section` referenced by newer threads and/or desktops. (TODO: is this actually an issue? What's the maximum bound for this "leak"? The number of threads?)
Instead of invalidating it on next query, I think we should be actively freeing them instead. To do this, we would replace `force = invalid = TRUE` with some kind of a weak-ref scheme.
Some review comments haven't been thoroughly vetted and are not final (I can change my mind later if I procure more time), so please keep this in mind. Thanks!
On Tue Feb 27 21:10:12 2024 +0000, Jinoh Kang wrote:
Some review comments haven't been thoroughly vetted and are not final (I can change my mind later if I get more time in my hands), so please keep this in mind. Thanks!
I've pushed some changes to address them tentatively to https://gitlab.winehq.org/rbernon/wine/-/commits/mr/shared-memories. They look reasonable to me and I intend to squash them if you think they're worth having.
-
Jinoh Kang (＠iamahuman) -
Rémi Bernon
-
Rémi Bernon (＠rbernon)