This patch set fixes most performance problems caused by ReleaseSemaphore() and WaitForSingle/MultipleObject(s) making server calls. One victim of this is Star Wars Battlefront (https://bugs.winehq.org/show_bug.cgi?id=29582), where the majority of the CPU time is spent on context switching while the program spams ReleaseSemaphore, WaitForSingle/MultipleObject(s) and GetForgroundWindow, the later of which I made a work-around hack for and that some players have been using.
This patch set doubles performance of bug #29582. (When combined with GetForgroundWindow hack the problem is completely resolved.) The patch set works by having the server create a POSIX semaphore object and sharing the key to that object with the client process, enabling the client process to be able to implement ReleaseSemaphore and optimistic-case wait calls (where no blocking is reburied) without a server call. Blocking waits and any wait-multiple that cannot be resolved in the client process (e.g., bWaitAll=TRUE and objects include non-semaphores) is still handled by the server. (Implementing blocking wait calls on the client can yield some performance improvements because a context switch to another thread in the same program won't require swapping out the memory map & such, but I would expect this to be less significant.)
However, upon further experimentation, I discovered that POSIX semaphores in glibc are actually implemented using a shared memory page, which may not be acceptable since a bad process can corrupt that page and potentially cause sem_* function calls in the server to fail as well as other client programs fail and/or deadlock. I am working on a System V adaptation, but I thought it would be a good idea to see feedback & comments now.
Another problem is that this causes the threadpool test fails at line 1299, where the previous "release all semaphores and wait for callback" test is done in reverse order. I presume this is due to the nature of the linux scheduler being inconsistent with how Windows *happens* to schedule its threads. I have an idea for a fix for this already, but I will still have to dig deeper into it.
The code is still in experimental quality (assert(0)s and such) and I've already re-worked the configure.ac stuff, I'm mostly concerned with feedback on the general scheme.
Thanks! Daniel
This is the switch that turns on the patchset via the config.h macro ENABLE_POSIX_SYNC.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- configure | 34 ++++++++++++++++++++++++++++++++++ configure.ac | 12 ++++++++++++ include/config.h.in | 6 ++++++ 3 files changed, 52 insertions(+)
diff --git a/configure b/configure index 20371dc..097b689 100755 --- a/configure +++ b/configure @@ -800,6 +800,7 @@ enable_win16 enable_win64 enable_tests enable_maintainer_mode +enable_hybrid_sync with_alsa with_capi with_cms @@ -2155,6 +2156,8 @@ Optional Features: --disable-tests do not build the regression tests --enable-maintainer-mode enable maintainer-specific build rules + --enable-hybrid-sync enable hybrid of server-side and process local + synchronisation objects (experimental) --disable-largefile omit support for large files
Optional Packages: @@ -3260,6 +3263,11 @@ if test "${enable_maintainer_mode+set}" = set; then : enableval=$enable_maintainer_mode; fi
+# Check whether --enable-hybrid-sync was given. +if test "${enable_hybrid_sync+set}" = set; then : + enableval=$enable_hybrid_sync; +fi +
# Check whether --with-alsa was given. @@ -7057,6 +7065,32 @@ fi done
+for ac_header in semaphore.h +do : + ac_fn_c_check_header_compile "$LINENO" "semaphore.h" "ac_cv_header_semaphore_h" "#ifdef HAVE_SEMAPHORE_H +#include <semaphore.h> +#endif +" +if test "x$ac_cv_header_semaphore_h" = xyes; then : + cat >>confdefs.h <<_ACEOF +#define HAVE_SEMAPHORE_H 1 +_ACEOF + +fi + +done + + +enable_posix_sync=0 +if test "x$enable_hybrid_sync" = "xyes" +then + +cat >>confdefs.h <<_ACEOF +#define ENABLE_POSIX_SYNC 1 +_ACEOF + +fi + for ac_header in linux/videodev.h linux/videodev2.h libv4l1.h do : as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh` diff --git a/configure.ac b/configure.ac index 1173190..e07ddf8 100644 --- a/configure.ac +++ b/configure.ac @@ -32,6 +32,7 @@ AC_ARG_ENABLE(win16, AS_HELP_STRING([--disable-win16],[do not include Win16 supp AC_ARG_ENABLE(win64, AS_HELP_STRING([--enable-win64],[build a Win64 emulator on AMD64 (won't run Win32 binaries)])) AC_ARG_ENABLE(tests, AS_HELP_STRING([--disable-tests],[do not build the regression tests])) AC_ARG_ENABLE(maintainer-mode, AS_HELP_STRING([--enable-maintainer-mode],[enable maintainer-specific build rules])) +AC_ARG_ENABLE(hybrid-sync, AS_HELP_STRING([--enable-hybrid-sync],[enable hybrid of server-side and process local synchronisation objects (experimental)]))
AC_ARG_WITH(alsa, AS_HELP_STRING([--without-alsa],[do not use the Alsa sound support]), [if test "x$withval" = "xno"; then ac_cv_header_sys_asoundlib_h=no; ac_cv_header_alsa_asoundlib_h=no; fi]) @@ -650,6 +651,17 @@ AC_CHECK_HEADERS([pthread_np.h],,, #include <pthread.h> #endif])
+AC_CHECK_HEADERS([semaphore.h],,, +[#ifdef HAVE_SEMAPHORE_H +#include <semaphore.h> +#endif]) + +enable_posix_sync=0 +if test "x$enable_hybrid_sync" = "xyes" +then + AC_DEFINE_UNQUOTED(ENABLE_POSIX_SYNC, 1, [Define to 1 to enable hybrid synchronization.]) +fi + AC_CHECK_HEADERS([linux/videodev.h linux/videodev2.h libv4l1.h],,, [#ifdef HAVE_SYS_TIME_H #include <sys/time.h> diff --git a/include/config.h.in b/include/config.h.in index eb61a94..6615a98 100644 --- a/include/config.h.in +++ b/include/config.h.in @@ -7,6 +7,9 @@ /* Define to a function attribute for Microsoft hotpatch assembly prefix. */ #undef DECLSPEC_HOTPATCH
+/* Define to 1 to enable hybrid synchronization. */ +#undef ENABLE_POSIX_SYNC + /* Define to the file extension for executables. */ #undef EXEEXT
@@ -759,6 +762,9 @@ /* Define to 1 if you have the `select' function. */ #undef HAVE_SELECT
+/* Define to 1 if you have the <semaphore.h> header file. */ +#undef HAVE_SEMAPHORE_H + /* Define to 1 if you have the `sendmsg' function. */ #undef HAVE_SENDMSG
When the server creates or opens a semaphore, the key is passed to the client so that they may open the same semaphore locally.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- include/wine/server_protocol.h | 10 +++++++--- server/protocol.def | 5 +++++ server/request.h | 9 +++++++-- server/trace.c | 5 +++++ 4 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 2ba71e8..81c99d2 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -1308,7 +1308,8 @@ struct create_semaphore_reply { struct reply_header __header; obj_handle_t handle; - char __pad_12[4]; + unsigned int key; + client_ptr_t server_ptr; };
@@ -1352,7 +1353,10 @@ struct open_semaphore_reply { struct reply_header __header; obj_handle_t handle; - char __pad_12[4]; + unsigned int max; + unsigned int key; + char __pad_20[4]; + client_ptr_t server_ptr; };
@@ -6149,6 +6153,6 @@ union generic_reply struct terminate_job_reply terminate_job_reply; };
-#define SERVER_PROTOCOL_VERSION 487 +#define SERVER_PROTOCOL_VERSION 488
#endif /* __WINE_WINE_SERVER_PROTOCOL_H */ diff --git a/server/protocol.def b/server/protocol.def index c313006..38bcca6 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1103,6 +1103,8 @@ enum event_op { PULSE_EVENT, SET_EVENT, RESET_EVENT }; VARARG(objattr,object_attributes); /* object attributes */ @REPLY obj_handle_t handle; /* handle to the semaphore */ + unsigned int key; /* key to the native semaphore */ + client_ptr_t server_ptr; /* for debugging only */ @END
@@ -1129,6 +1131,9 @@ enum event_op { PULSE_EVENT, SET_EVENT, RESET_EVENT }; VARARG(name,unicode_str); /* object name */ @REPLY obj_handle_t handle; /* handle to the semaphore */ + unsigned int max; /* maximum count */ + unsigned int key; /* key to the native semaphore */ + client_ptr_t server_ptr; /* for debugging only */ @END
diff --git a/server/request.h b/server/request.h index 9405179..bfa4ef9 100644 --- a/server/request.h +++ b/server/request.h @@ -918,7 +918,9 @@ C_ASSERT( FIELD_OFFSET(struct create_semaphore_request, initial) == 20 ); C_ASSERT( FIELD_OFFSET(struct create_semaphore_request, max) == 24 ); C_ASSERT( sizeof(struct create_semaphore_request) == 32 ); C_ASSERT( FIELD_OFFSET(struct create_semaphore_reply, handle) == 8 ); -C_ASSERT( sizeof(struct create_semaphore_reply) == 16 ); +C_ASSERT( FIELD_OFFSET(struct create_semaphore_reply, key) == 12 ); +C_ASSERT( FIELD_OFFSET(struct create_semaphore_reply, server_ptr) == 16 ); +C_ASSERT( sizeof(struct create_semaphore_reply) == 24 ); C_ASSERT( FIELD_OFFSET(struct release_semaphore_request, handle) == 12 ); C_ASSERT( FIELD_OFFSET(struct release_semaphore_request, count) == 16 ); C_ASSERT( sizeof(struct release_semaphore_request) == 24 ); @@ -934,7 +936,10 @@ C_ASSERT( FIELD_OFFSET(struct open_semaphore_request, attributes) == 16 ); C_ASSERT( FIELD_OFFSET(struct open_semaphore_request, rootdir) == 20 ); C_ASSERT( sizeof(struct open_semaphore_request) == 24 ); C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, handle) == 8 ); -C_ASSERT( sizeof(struct open_semaphore_reply) == 16 ); +C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, max) == 12 ); +C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, key) == 16 ); +C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, server_ptr) == 24 ); +C_ASSERT( sizeof(struct open_semaphore_reply) == 32 ); C_ASSERT( FIELD_OFFSET(struct create_file_request, access) == 12 ); C_ASSERT( FIELD_OFFSET(struct create_file_request, attributes) == 16 ); C_ASSERT( FIELD_OFFSET(struct create_file_request, sharing) == 20 ); diff --git a/server/trace.c b/server/trace.c index 07aee80..fbaa426 100644 --- a/server/trace.c +++ b/server/trace.c @@ -1592,6 +1592,8 @@ static void dump_create_semaphore_request( const struct create_semaphore_request static void dump_create_semaphore_reply( const struct create_semaphore_reply *req ) { fprintf( stderr, " handle=%04x", req->handle ); + fprintf( stderr, ", key=%08x", req->key ); + dump_uint64( ", server_ptr=", &req->server_ptr ); }
static void dump_release_semaphore_request( const struct release_semaphore_request *req ) @@ -1627,6 +1629,9 @@ static void dump_open_semaphore_request( const struct open_semaphore_request *re static void dump_open_semaphore_reply( const struct open_semaphore_reply *req ) { fprintf( stderr, " handle=%04x", req->handle ); + fprintf( stderr, ", max=%08x", req->max ); + fprintf( stderr, ", key=%08x", req->key ); + dump_uint64( ", server_ptr=", &req->server_ptr ); }
static void dump_create_file_request( const struct create_file_request *req )
Changes server to use a posix semaphore instead of an int. This patch alone would be pointless, but creates a mechanism for client processes to interact with the same semaphores.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- server/Makefile.in | 2 +- server/object.h | 4 + server/semaphore.c | 272 +++++++++++++++++++++++++++++++++++++++++++++++------ server/thread.c | 36 ++++++- 4 files changed, 280 insertions(+), 34 deletions(-)
diff --git a/server/Makefile.in b/server/Makefile.in index 19a4fac..48a4522 100644 --- a/server/Makefile.in +++ b/server/Makefile.in @@ -1,4 +1,4 @@ -EXTRALIBS = $(POLL_LIBS) $(RT_LIBS) +EXTRALIBS = $(POLL_LIBS) $(RT_LIBS) $(PTHREAD_LIBS)
C_SRCS = \ async.c \ diff --git a/server/object.h b/server/object.h index b59811f..fc650ff 100644 --- a/server/object.h +++ b/server/object.h @@ -90,6 +90,10 @@ struct object_ops int (*close_handle)(struct object *,struct process *,obj_handle_t); /* destroy on refcount == 0 */ void (*destroy)(struct object *); + /* like signaled(), but tries to get the lock */ + int (*trylock)(struct object *,struct wait_queue_entry *); + /* undo a previously sucessful call to trylock */ + void (*trylock_undo)(struct object *,struct wait_queue_entry *); };
struct object diff --git a/server/semaphore.c b/server/semaphore.c index d87325c..3c8dd14 100644 --- a/server/semaphore.c +++ b/server/semaphore.c @@ -26,6 +26,21 @@ #include <stdlib.h> #include <stdarg.h>
+#if ENABLE_POSIX_SYNC +# include <fcntl.h> +# include <sys/stat.h> +# include <errno.h> +# ifdef HAVE_SEMAPHORE_H +# include <semaphore.h> +# endif +# ifdef HAVE_SYS_TYPES_H +# include <sys/types.h> +# endif +# ifdef HAVE_UNISTD_H +# include <unistd.h> +# endif +#endif + #include "ntstatus.h" #define WIN32_NO_STATUS #include "windef.h" @@ -36,10 +51,17 @@ #include "request.h" #include "security.h"
+#define NATIVE_SEMAPHORE_MAX_NAME (32) struct semaphore { struct object obj; /* object header */ +#if ENABLE_POSIX_SYNC + sem_t *p; + unsigned int key; + char name[NATIVE_SEMAPHORE_MAX_NAME]; +#else unsigned int count; /* current count */ +#endif /* ENABLE_POSIX_SYNC */ unsigned int max; /* maximum possible count */ };
@@ -49,6 +71,15 @@ static int semaphore_signaled( struct object *obj, struct wait_queue_entry *entr static void semaphore_satisfied( struct object *obj, struct wait_queue_entry *entry ); static unsigned int semaphore_map_access( struct object *obj, unsigned int access ); static int semaphore_signal( struct object *obj, unsigned int access ); +static void semaphore_new( struct semaphore *sem, unsigned int initial ); +static unsigned int semaphore_get_value( struct semaphore *sem ); +//static void semaphore_up( struct semaphore *sem, unsigned int count); + +#if ENABLE_POSIX_SYNC +static void semaphore_destroy( struct object *obj ); +static int semaphore_trylock( struct object *obj, struct wait_queue_entry *entry ); +static void semaphore_trylock_undo( struct object *obj, struct wait_queue_entry *entry ); +#endif /* ENABLE_POSIX_SYNC */
static const struct object_ops semaphore_ops = { @@ -67,35 +98,32 @@ static const struct object_ops semaphore_ops = no_lookup_name, /* lookup_name */ no_open_file, /* open_file */ no_close_handle, /* close_handle */ - no_destroy /* destroy */ +#if ENABLE_POSIX_SYNC + semaphore_destroy, /* destroy */ + semaphore_trylock, /* trylock */ + semaphore_trylock_undo /* trylock_undo */ +#else + no_destroy, /* destroy */ + NULL, /* trylock */ + NULL /* trylock_undo */ +#endif };
+#if !(ENABLE_POSIX_SYNC)
-static struct semaphore *create_semaphore( struct directory *root, const struct unicode_str *name, - unsigned int attr, unsigned int initial, unsigned int max, - const struct security_descriptor *sd ) +static inline void semaphore_new( struct semaphore *sem, unsigned int initial ) { - struct semaphore *sem; + sem->count = initial; +}
- if (!max || (initial > max)) - { - set_error( STATUS_INVALID_PARAMETER ); - return NULL; - } - if ((sem = create_named_object_dir( root, name, attr, &semaphore_ops ))) - { - if (get_error() != STATUS_OBJECT_NAME_EXISTS) - { - /* initialize it if it didn't already exist */ - sem->count = initial; - sem->max = max; - if (sd) default_set_sd( &sem->obj, sd, OWNER_SECURITY_INFORMATION| - GROUP_SECURITY_INFORMATION| - DACL_SECURITY_INFORMATION| - SACL_SECURITY_INFORMATION ); - } - } - return sem; +static inline unsigned int semaphore_get_value( struct semaphore *sem ) +{ + return sem->count; +} + +static inline void semaphore_down( struct semaphore *sem ) +{ + --sem->count; }
static int release_semaphore( struct semaphore *sem, unsigned int count, @@ -119,12 +147,187 @@ static int release_semaphore( struct semaphore *sem, unsigned int count, } return 1; } +#else /* !(ENABLE_POSIX_SYNC) */ + +static DWORD next_semaphore_id = 0; + +static void semaphore_new( struct semaphore *sem, unsigned int initial ) +{ + /* imperfect, but good enough for now */ + int tries; + for (tries = 0; tries < 0x10000; ++tries) + { + DWORD pid = (DWORD)getpid(); + DWORD reverse_pid = 0; + DWORD bit; + + for (bit = (1 << (sizeof(DWORD) * 8 - 1)); bit && pid; bit >>= 1, pid >>= 1) + { + if (pid & 1) + reverse_pid |= bit; + } + + sem->key = reverse_pid ^ next_semaphore_id++; + snprintf(sem->name, sizeof(sem->name) - 1, "/wine-sem-%08x", sem->key); + sem->p = sem_open(sem->name, O_CREAT | O_EXCL, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP, initial); + + if (sem->p != SEM_FAILED) + { + //fprintf(stderr, "semaphore_new: sem = %p, sem->p = %p, sem->name = %s\n", sem, sem->p, sem->name); + return; + } + + switch (errno) + { + case EACCES: + case EEXIST: + continue; + + case EINVAL: + set_error( STATUS_INVALID_PARAMETER ); + break; + case EMFILE: + case ENFILE: + set_error( STATUS_TOO_MANY_OPENED_FILES ); + break; + case ENOMEM: + set_error( STATUS_NO_MEMORY ); + break; + default: + break; + } + perror("sem_open"); + assert(0); + exit(1); + } + fprintf(stderr, "failed to create unique key for semaphore\n"); + assert(0); +} + +static unsigned int semaphore_get_value( struct semaphore *sem ) +{ + int ret; + + if (sem_getvalue(sem->p, &ret) == -1) + { + perror("sem_getvalue"); + exit(1); + + return STATUS_INVALID_HANDLE; + } + + return (unsigned int)ret; +} + +static void semaphore_destroy( struct object *obj ) +{ + struct semaphore *sem = (struct semaphore *)obj; + assert( obj->ops == &semaphore_ops ); + sem_destroy(sem->p); +} + +static int semaphore_trylock( struct object *obj, struct wait_queue_entry *entry ) +{ + struct semaphore *sem = (struct semaphore *)obj; + assert( obj->ops == &semaphore_ops ); + if (sem_trywait(sem->p) == -1) + { + switch(errno) { + case EAGAIN: + return 0; + + default: + perror("sem_trywait"); + exit(1); + } + } + return 1; +} + +static void semaphore_trylock_undo( struct object *obj, struct wait_queue_entry *entry ) +{ + struct semaphore *sem = (struct semaphore *)obj; + assert( obj->ops == &semaphore_ops ); + if (sem_post(sem->p) == -1) + { + perror("sem_post"); + exit(1); + } + +} + +static int release_semaphore( struct semaphore *sem, unsigned int count, + unsigned int *prev ) +{ + unsigned int cur_count; + + cur_count = semaphore_get_value(sem); + + if (prev) *prev = cur_count; + if (cur_count + count < cur_count || cur_count + count > sem->max) + { + set_error( STATUS_SEMAPHORE_LIMIT_EXCEEDED ); + return 0; + } + else + { + while(count--) + { + if (sem_post(sem->p) == -1) + { + /* FIXME: no atomic way to increase more than once */ + perror("sem_post"); + } + } + + /* there cannot be any thread to wake up if the count is != 0 */ + if (!cur_count) + wake_up( &sem->obj, count ); + } + return 1; +} + +#endif /* ENABLE_POSIX_SYNC */ + +static struct semaphore *create_semaphore( struct directory *root, const struct unicode_str *name, + unsigned int attr, unsigned int initial, unsigned int max, + const struct security_descriptor *sd ) +{ + struct semaphore *sem; + + if (!max || (initial > max)) + { + set_error( STATUS_INVALID_PARAMETER ); + return NULL; + } + if ((sem = create_named_object_dir( root, name, attr, &semaphore_ops ))) + { + if (get_error() != STATUS_OBJECT_NAME_EXISTS) + { + /* initialize it if it didn't already exist */ + semaphore_new(sem, initial); + sem->max = max; + if (sd) default_set_sd( &sem->obj, sd, OWNER_SECURITY_INFORMATION| + GROUP_SECURITY_INFORMATION| + DACL_SECURITY_INFORMATION| + SACL_SECURITY_INFORMATION ); + } +#if ENABLE_POSIX_SYNC + else + { + assert(sem->p); + assert(sem->name[0]); + } +#endif + } + return sem; +}
static void semaphore_dump( struct object *obj, int verbose ) { struct semaphore *sem = (struct semaphore *)obj; assert( obj->ops == &semaphore_ops ); - fprintf( stderr, "Semaphore count=%d max=%d ", sem->count, sem->max ); + fprintf( stderr, "Semaphore count=%d max=%d ", semaphore_get_value(sem), sem->max ); dump_object_name( &sem->obj ); fputc( '\n', stderr ); } @@ -140,15 +343,17 @@ static int semaphore_signaled( struct object *obj, struct wait_queue_entry *entr { struct semaphore *sem = (struct semaphore *)obj; assert( obj->ops == &semaphore_ops ); - return (sem->count > 0); + return (semaphore_get_value(sem) > 0); }
static void semaphore_satisfied( struct object *obj, struct wait_queue_entry *entry ) { +#ifndef ENABLE_POSIX_SYNC struct semaphore *sem = (struct semaphore *)obj; assert( obj->ops == &semaphore_ops ); - assert( sem->count ); - sem->count--; + assert( semaphore_get_value(sem) ); + semaphore_down(sem); +#endif }
static unsigned int semaphore_map_access( struct object *obj, unsigned int access ) @@ -199,6 +404,10 @@ DECL_HANDLER(create_semaphore) reply->handle = alloc_handle( current->process, sem, req->access, req->attributes ); else reply->handle = alloc_handle_no_access_check( current->process, sem, req->access, req->attributes ); +#ifdef ENABLE_POSIX_SYNC + reply->key = sem->key; +#endif + reply->server_ptr = (unsigned long)sem; release_object( sem ); }
@@ -219,6 +428,11 @@ DECL_HANDLER(open_semaphore) if ((sem = open_object_dir( root, &name, req->attributes, &semaphore_ops ))) { reply->handle = alloc_handle( current->process, &sem->obj, req->access, req->attributes ); +#ifdef ENABLE_POSIX_SYNC + reply->key = sem->key; +#endif + reply->max = sem->max; + reply->server_ptr = (unsigned long)sem; release_object( sem ); }
@@ -246,7 +460,7 @@ DECL_HANDLER(query_semaphore) if ((sem = (struct semaphore *)get_handle_obj( current->process, req->handle, SEMAPHORE_QUERY_STATE, &semaphore_ops ))) { - reply->current = sem->count; + reply->current = semaphore_get_value(sem); reply->max = sem->max; release_object( sem ); } diff --git a/server/thread.c b/server/thread.c index 6383000..4ed39d6 100644 --- a/server/thread.c +++ b/server/thread.c @@ -653,6 +653,12 @@ static int check_wait( struct thread *thread ) int i; struct thread_wait *wait = thread->wait; struct wait_queue_entry *entry; + const int posix_sync_enabled = +#ifdef ENABLE_POSIX_SYNC + 1; +#else + 0; +#endif
assert( wait );
@@ -664,12 +670,30 @@ static int check_wait( struct thread *thread )
if (wait->select == SELECT_WAIT_ALL) { - int not_ok = 0; + ULONGLONG ok = 0; + assert(MAXIMUM_WAIT_OBJECTS <= sizeof(ULONGLONG) * 8); /* Note: we must check them all anyway, as some objects may * want to do something when signaled, even if others are not */ for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) - not_ok |= !entry->obj->ops->signaled( entry->obj, entry ); - if (not_ok) goto other_checks; + { + int signaled = posix_sync_enabled && entry->obj->ops->trylock + ? entry->obj->ops->trylock( entry->obj, entry ) + : entry->obj->ops->signaled( entry->obj, entry ); + ok |= (ULONGLONG)(!!signaled) << i; + } + if (!ok) + { + if (posix_sync_enabled) + { + /* reverse iterate and undo any sucessful trylocks */ + for (--i, --entry; i >= 0; --i, --entry) + { + if ((ok & (1LL << i)) && entry->obj->ops->trylock) + entry->obj->ops->trylock_undo( entry->obj, entry ); + } + } + goto other_checks; + } /* Wait satisfied: tell it to all objects */ for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) entry->obj->ops->satisfied( entry->obj, entry ); @@ -679,7 +703,11 @@ static int check_wait( struct thread *thread ) { for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) { - if (!entry->obj->ops->signaled( entry->obj, entry )) continue; + int signaled = posix_sync_enabled && entry->obj->ops->trylock + ? entry->obj->ops->trylock( entry->obj, entry ) + : entry->obj->ops->signaled( entry->obj, entry ); + + if (!signaled) continue; /* Wait satisfied: tell it to the object */ entry->obj->ops->satisfied( entry->obj, entry ); if (wait->abandoned) i += STATUS_ABANDONED_WAIT_0;
I apparently need a new email provider. My SMTP server is just dropping the connection when I try to send the rest of the patches. :( Sorry about that, I'll send the rest as soon as I am able to.
When a semaphore is opened or created, the client process now also opens that semaphore and can perform operations on it
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- dlls/ntdll/sync.c | 232 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 232 insertions(+)
diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c index 6892732..84a3ab6 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -48,6 +48,19 @@ #include <stdlib.h> #include <time.h>
+#if ENABLE_POSIX_SYNC +# ifdef HAVE_SEMAPHORE_H +# include <semaphore.h> +# endif +# include <fcntl.h> +# include <sys/stat.h> +# include <semaphore.h> +# include <errno.h> +# ifdef HAVE_SYS_TYPES_H +# include <sys/types.h> +# endif +#endif + #include "ntstatus.h" #define WIN32_NO_STATUS #define NONAMELESSUNION @@ -58,9 +71,226 @@ #include "ntdll_misc.h"
WINE_DEFAULT_DEBUG_CHANNEL(ntdll); +WINE_DECLARE_DEBUG_CHANNEL(ntdllsync);
HANDLE keyed_event = NULL;
+static NTSTATUS semaphore_add(const HANDLE h, LONG MaximumCount, unsigned int key, unsigned long debug_server_ptr); + +#if !(ENABLE_POSIX_SYNC) +static NTSTATUS semaphore_add(const HANDLE h, LONG MaximumCount, unsigned int key, unsigned long debug_server_ptr) +{ + return STATUS_SUCCESS; +} + +static inline NTSTATUS semaphore_up(const HANDLE h, ULONG count, PULONG previous) +{ + return STATUS_NOT_IMPLEMENTED; +} +#else /* ENABLE_POSIX_SYNC */ + +struct ntdll_semaphore { + struct ntdll_object obj; + sem_t *p; + LONG max; + unsigned int key; + unsigned long server_ptr; +}; + +static void semaphore_dump(const struct ntdll_object *obj, char **start, const char *const end); + +#if 0 +static int sem_ +sem_getvalue + if (sem_getvalue(sem->p, &ret) == -1) + { + perror("sem_getvalue"); + exit(1); + + return STATUS_INVALID_HANDLE; + } + + return (unsigned int)ret; +} +#endif + +static NTSTATUS semaphore_up(struct ntdll_semaphore *sem, ULONG count, PULONG previous) +{ + int current_value; + + TRACE_(ntdllsync)("(sem = %s, count = %u, previous = %p)\n", + ntdll_object_dump(&sem->obj), count, previous); + + if (sem_getvalue(sem->p, ¤t_value) == -1) + { + perror("sem_getvalue"); + assert(0); + ERR_(ntdllsync)("sem_getvalue failed.\n"); + + return STATUS_INVALID_HANDLE; /* ?? */ + } + + if (previous) + *previous = current_value; + + if (current_value + count > sem->max) + return STATUS_SEMAPHORE_LIMIT_EXCEEDED; + + while (count--) + { + /* BUG by multiple threads releasing at the same time, it is possible to exceed sem->max here */ + if (sem_post(sem->p) == -1) + { + perror("sem_post"); + /* ?? */ + } + } + TRACE_(ntdllsync)("success\n"); + + ntdll_server_notify_lock_release(); + return STATUS_SUCCESS; +} + +/* client-side waiting not yet implemented */ +static NTSTATUS semaphore_wait(struct ntdll_object *obj, const LARGE_INTEGER *timeout) +{ + TRACE_(ntdllsync)("(obj = %s, timeout = %p)\n", ntdll_object_dump(obj), timeout); + + return STATUS_NOT_IMPLEMENTED; +} + +static NTSTATUS semaphore_trywait(struct ntdll_object *obj) +{ + struct ntdll_semaphore *sem = (void *)obj; + + TRACE_(ntdllsync)("(obj = %s)\n", ntdll_object_dump(obj)); + + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + assert(sem->p); + + if (sem_trywait(sem->p)) + { + switch (errno) { + case EAGAIN: + return STATUS_WAS_LOCKED; + case EDEADLK: + return STATUS_POSSIBLE_DEADLOCK; + default: + perror("sem_trywait"); + exit(1); + } + } + return STATUS_SUCCESS; +} + +static NTSTATUS semaphore_trywait_undo(struct ntdll_object *obj) +{ + struct ntdll_semaphore *sem = (void *)obj; + + TRACE_(ntdllsync)("(obj = %s)\n", ntdll_object_dump(obj)); + + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + assert(sem->p); + + if (sem_post(sem->p) == -1) + { + perror("sem_post"); + exit(1); + } + return STATUS_SUCCESS; +} + +static void semaphore_close(struct ntdll_object *obj) +{ + struct ntdll_semaphore *sem = (void *)obj; + + TRACE_(ntdllsync)("(obj = %s)\n", ntdll_object_dump(obj)); + + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + assert(sem->p); + + if (sem->p) + sem_close(sem->p); +} + +static void semaphore_dump(const struct ntdll_object *obj, char **start, const char *const end) +{ + struct ntdll_semaphore *sem = (void *)obj; + int count; + int sem_value; + + assert(sem); + assert(sem->p); + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + + count = snprintf(*start, end - *start, "%p {obj = ", obj); + if (count < 0) + { + perror("snprintf"); + return; + } + *start += count; + + ntdll_object_dump_base(obj, start, end); + + if (sem_getvalue(sem->p, &sem_value) == -1) + sem_value = -1; + + count = snprintf(*start, end - *start, + ", p = %p {value = %d}, max = %d, key = %08x, server_ptr = %p", + sem->p, sem_value, sem->max, sem->key, (void*)sem->server_ptr); + if (count < 0) + perror("snprintf"); +} + +static struct ntdll_object_ops semaphore_ops = { + semaphore_wait, /* wait */ + semaphore_trywait, /* trywait */ + semaphore_trywait_undo, /* trywait_undo */ + semaphore_close, /* close */ + semaphore_dump /* dump */ +}; + +static NTSTATUS semaphore_add(const HANDLE h, LONG MaximumCount, unsigned int key, unsigned long server_ptr) +{ + struct ntdll_semaphore *sem; + sem_t *sem_obj; + char name[32]; + + snprintf(name, sizeof(name) - 1, "/wine-sem-%08x", key); + TRACE_(ntdllsync)("(h = %p, MaximumCount = %d, key = %08x, name = "%s")\n", h, MaximumCount, key, name); + sem_obj = sem_open(name, 0); + + if (sem_obj == SEM_FAILED) + { + perror("sem_open"); + assert(0); + exit(1); + } + + sem = (void *)ntdll_object_new(h, sizeof(struct ntdll_semaphore), NTDLL_OBJ_TYPE_SEMAPHORE, &semaphore_ops); + + if (!sem) + { + ERR_(ntdllsync)("ntdll_object_new failed\n"); + sem_close(sem_obj); + return STATUS_NO_MEMORY; + } + sem->max = MaximumCount; + sem->p = sem_obj; + sem->key = key; + sem->server_ptr = server_ptr; + TRACE_(ntdllsync)("%s\n", ntdll_object_dump(&sem->obj)); + + ntdll_handle_add(&sem->obj); + ntdll_object_release(&sem->obj); + + return STATUS_SUCCESS; +} + +#endif /* ENABLE_POSIX_SYNC */ + + static inline int interlocked_dec_if_nonzero( int *dest ) { int val, tmp; @@ -184,6 +414,7 @@ NTSTATUS WINAPI NtCreateSemaphore( OUT PHANDLE SemaphoreHandle, if (len) wine_server_add_data( req, attr->ObjectName->Buffer, len ); ret = wine_server_call( req ); *SemaphoreHandle = wine_server_ptr_handle( reply->handle ); + semaphore_add(*SemaphoreHandle, MaximumCount, reply->key, (unsigned long)reply->server_ptr); } SERVER_END_REQ;
@@ -212,6 +443,7 @@ NTSTATUS WINAPI NtOpenSemaphore( OUT PHANDLE SemaphoreHandle, if (len) wine_server_add_data( req, attr->ObjectName->Buffer, len ); ret = wine_server_call( req ); *SemaphoreHandle = wine_server_ptr_handle( reply->handle ); + semaphore_add(*SemaphoreHandle, reply->max, reply->key, (unsigned long)reply->server_ptr); } SERVER_END_REQ; return ret;
When enabled, ReleaseSemaphore will be done by the client process without a server call. However, notification to the server is required and a rather ugly but temporary SIGUSR1 mechanism is implemented (will be changed soon).
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- dlls/ntdll/server.c | 14 ++++++++++++++ dlls/ntdll/sync.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+)
diff --git a/dlls/ntdll/server.c b/dlls/ntdll/server.c index 95111ad..763a90d 100644 --- a/dlls/ntdll/server.c +++ b/dlls/ntdll/server.c @@ -682,6 +682,20 @@ unsigned int server_queue_process_apc( HANDLE process, const apc_call_t *call, a } }
+/* notify server that we've released a lock as it won't actually wait (block) + * on any locks and may have a thread suspended waiting for this lock to be + * released */ +void ntdll_server_notify_lock_release(void) +{ + TRACE_(ntdllsync)("server_pid = %u\n", server_pid); + if (server_pid && server_pid != -1) + { + if (kill(server_pid, SIGUSR1)) + { + perror("kill"); + } + } +}
/*********************************************************************** * wine_server_send_fd (NTDLL.@) diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c index 84a3ab6..a26e40c 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -487,6 +487,37 @@ NTSTATUS WINAPI NtQuerySemaphore( HANDLE handle, SEMAPHORE_INFORMATION_CLASS cla NTSTATUS WINAPI NtReleaseSemaphore( HANDLE handle, ULONG count, PULONG previous ) { NTSTATUS ret; + + struct ntdll_semaphore *sem = (void *)ntdll_handle_find(handle); + + /* if we know this handle, then manage it client-side, otherwise, it may be a stale handle + * that another thread already closed */ + if (sem) + { + TRACE_(ntdllsync)("handle = %p, count = %u, previous = %p) sem = %s\n", + handle, count, previous, ntdll_object_dump(&sem->obj)); + + assert(sem->p); + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + + ret = semaphore_up(sem, count, previous); + ntdll_object_release(&sem->obj); + + switch (ret) + { + case STATUS_SUCCESS: + return STATUS_SUCCESS; + + case STATUS_NOT_IMPLEMENTED: + /* fall through to server call */ + break; + + default: + ERR("semaphore_up return %08x\n", ret); + return ret; + } + } + SERVER_START_REQ( release_semaphore ) { req->handle = wine_server_obj_handle( handle );
Sever will now handle SIGUSR1 as notifcation that a client process has released a lock and will walk all threads to see if any are ready to run. This will be replaced by a proper asynchronous message where the client will tell the server exactly which object was released.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- server/signal.c | 12 ++++++++++++ server/thread.c | 20 ++++++++++++++++++++ server/thread.h | 1 + 3 files changed, 33 insertions(+)
diff --git a/server/signal.c b/server/signal.c index 5e4fe33..f241604 100644 --- a/server/signal.c +++ b/server/signal.c @@ -98,6 +98,7 @@ static struct handler *handler_sigterm; static struct handler *handler_sigint; static struct handler *handler_sigchld; static struct handler *handler_sigio; +static struct handler *handler_sigusr1;
static int watchdog;
@@ -239,6 +240,11 @@ static void do_sigio( int signum, siginfo_t *si, void *x ) } #endif
+static void do_sigusr1( int signum ) +{ + do_signal( handler_sigusr1 ); +} + void start_watchdog(void) { alarm( 3 ); @@ -277,6 +283,7 @@ void init_signals(void) if (!(handler_sigint = create_handler( sigint_callback ))) goto error; if (!(handler_sigchld = create_handler( sigchld_callback ))) goto error; if (!(handler_sigio = create_handler( sigio_callback ))) goto error; + if (!(handler_sigusr1 = create_handler( sigusr1_callback ))) goto error;
sigemptyset( &blocked_sigset ); sigaddset( &blocked_sigset, SIGCHLD ); @@ -289,6 +296,7 @@ void init_signals(void) #ifdef SIG_PTHREAD_CANCEL sigaddset( &blocked_sigset, SIG_PTHREAD_CANCEL ); #endif + sigaddset( &blocked_sigset, SIGUSR1 );
action.sa_mask = blocked_sigset; action.sa_flags = 0; @@ -318,6 +326,10 @@ void init_signals(void) action.sa_flags = SA_SIGINFO; sigaction( SIGIO, &action, NULL ); #endif + action.sa_handler = do_sigusr1; + action.sa_flags = 0; + sigaction( SIGUSR1, &action, NULL ); + return;
error: diff --git a/server/thread.c b/server/thread.c index 4ed39d6..b4edfd9 100644 --- a/server/thread.c +++ b/server/thread.c @@ -721,6 +721,26 @@ static int check_wait( struct thread *thread ) return -1; }
+/* a less-than-elegant solution to waking up threads after a thread releases a native + * lock and sends us SIGUSR1 */ +static int quick_n_dirty_check_wait_threads(struct process *p, void *smart_dummy) +{ + struct thread *t; + + LIST_FOR_EACH_ENTRY( t, &p->thread_list, struct thread, proc_entry ) + { + if (t->wait) + wake_thread ( t ); + } + + return 0; +} + +void sigusr1_callback(void) +{ + enum_processes(quick_n_dirty_check_wait_threads, NULL); +} + /* send the wakeup signal to a thread */ static int send_thread_wakeup( struct thread *thread, client_ptr_t cookie, int signaled ) { diff --git a/server/thread.h b/server/thread.h index 2821991..40f3e2f 100644 --- a/server/thread.h +++ b/server/thread.h @@ -127,6 +127,7 @@ extern struct thread_snapshot *thread_snap( int *count ); extern struct token *thread_get_impersonation_token( struct thread *thread ); extern int set_thread_affinity( struct thread *thread, affinity_t affinity ); extern int is_cpu_supported( enum cpu_type cpu ); +extern void sigusr1_callback(void);
/* ptrace functions */
server_select can now trywait on native semaphores in a select_op when wait conditions do not require the thread to block. Otherwise, a standard server call is made.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- dlls/ntdll/server.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+)
diff --git a/dlls/ntdll/server.c b/dlls/ntdll/server.c index 763a90d..997954c 100644 --- a/dlls/ntdll/server.c +++ b/dlls/ntdll/server.c @@ -83,6 +83,7 @@ #include "ntdll_misc.h"
WINE_DEFAULT_DEBUG_CHANNEL(server); +WINE_DECLARE_DEBUG_CHANNEL(ntdllsync);
/* Some versions of glibc don't define this */ #ifndef SCM_RIGHTS @@ -594,6 +595,136 @@ unsigned int server_select( const select_op_t *select_op, data_size_t size, UINT
memset( &result, 0, sizeof(result) );
+#if ENABLE_POSIX_SYNC + TRACE_(ntdllsync)("select_op = %p, size = %u, flags = 0x%x, timeout = %p (%zd)\n", + select_op, size, flags, timeout, (timeout ? timeout->QuadPart : 0)); + + if (select_op && (select_op->op == SELECT_WAIT || select_op->op == SELECT_WAIT_ALL)) + { + struct ntdll_object *objs[MAXIMUM_WAIT_OBJECTS]; + size_t nb_locally_lockable_objs = 0; + size_t nb_handles = (size - offsetof( select_op_t, wait.handles )) + / sizeof(select_op->wait.handles[0]); + ssize_t i; + NTSTATUS result; + NTSTATUS ret = STATUS_UNSUCCESSFUL; + + /* count the number of locally lockable objects & cache their pointers */ + for (i = 0; i < nb_handles; ++i) + { + objs[i] = ntdll_handle_find(wine_server_ptr_handle(select_op->wait.handles[i])); + if (objs[i] && objs[i]->ops->trywait) + ++nb_locally_lockable_objs; + } + + TRACE_(ntdllsync)("%zd/%zd objects are locally selectable, need %s.\n", + nb_locally_lockable_objs, nb_handles, + (select_op->op == SELECT_WAIT ? "any" : "all")); + + /* bWaitAll = FALSE */ + if (select_op->op == SELECT_WAIT) + { + if (!nb_locally_lockable_objs) + goto local_done; + + /* waitformultipleobjectsex with bWaitAll = FALSE must go in order */ + for (i = 0; i < nb_handles; ++i) + { + /* If we can't lock this one locally, we must do the server call */ + if (!objs[i] || !objs[i]->ops->trywait) + break; + + result = objs[i]->ops->trywait(objs[i]); + if (result == STATUS_SUCCESS) + { + TRACE_(ntdllsync)("Successful local wait any. obj = %p (h = %p)\n", objs[i], objs[i]->h); + + ret = STATUS_SUCCESS; + break; + } + else if (result == STATUS_WAS_LOCKED) + continue; + else + ret = result; + } + } + + else /* select_op->op == SELECT_WAIT_ALL (bWaitAll = TRUE)*/ + { + ULONGLONG locked_objs = 0; + assert(MAXIMUM_WAIT_OBJECTS <= sizeof(locked_objs) * 8); + + if (nb_locally_lockable_objs != nb_handles) + goto local_done; + + result = 0; + + /* try to lock all objects */ + for (i = 0; i < nb_handles; ++i) + { + result = objs[i]->ops->trywait(objs[i]); + + if (result == STATUS_WAS_LOCKED) /* normal trywait failure */ + break; + else if (result != STATUS_POSSIBLE_DEADLOCK) + { + WARN("Possible deadlock in thread 0x%x, waiting on handle %p\n", GetCurrentThreadId(), objs[i]->h); + break; + } + else if (result != STATUS_SUCCESS) + { + ERR("object (handle = %p) trywait returned 0x%x\n", objs[i]->h, result); + break; + } + + /* keep track of the objects we've locked, max 64 */ + locked_objs |= 1LL << i; + TRACE_(ntdllsync)("locked object %zd/%zd\n", i, nb_handles); + } + + /* This is a little redundant, if result == STATUS_SUCCESS, then i should also be == nb_handles */ + if (result == STATUS_SUCCESS && i == nb_handles) + { + TRACE_(ntdllsync)("Successful local wait all. count = %zu\n", nb_handles); + ret = STATUS_SUCCESS; + } + /* if not successful then roll back locks */ + else + { + TRACE_(ntdllsync)("rolling back...\n"); + for (; i >= 0; --i) + { + TRACE_(ntdllsync)("rolling back %zd/%zd\n", i, nb_handles); + if (locked_objs & (1 << i)) + objs[i]->ops->trywait_undo(objs[i]); + } + + if (result != STATUS_WAS_LOCKED) + return result; + } + } + +local_done: + + /* release any objects we've grabbed */ + for (i = 0; i < nb_handles; ++i) + if (objs[i]) + ntdll_object_release(objs[i]); + + switch (ret) + { + case STATUS_SUCCESS: + return STATUS_SUCCESS; + + default: + /* fall through to server call */ + break; + } + } +TRACE_(ntdllsync)("Doing server call.\n"); + +#endif /* ENABLE_POSIX_SYNC */ + for (;;) { SERVER_START_REQ( select )
Adds a facility for ntdll to track handles locally and attach information to them, allowing for some operations to be performed directly by the client process.
(I had to split this patch up due to SMTP server, sorry)
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- dlls/ntdll/loader.c | 1 + dlls/ntdll/ntdll_misc.h | 61 +++++++++++ dlls/ntdll/om.c | 264 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 326 insertions(+)
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 8a43310..0c779e2 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -3258,6 +3258,7 @@ void __wine_process_init(void) umask( FILE_umask );
load_global_options(); + ntdll_object_db_init();
/* setup the load callback and create ntdll modref */ wine_dll_set_callback( load_builtin_callback ); diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h index cbd19db..08e48d7 100644 --- a/dlls/ntdll/ntdll_misc.h +++ b/dlls/ntdll/ntdll_misc.h @@ -28,6 +28,8 @@ #include "winnt.h" #include "winternl.h" #include "wine/server.h" +#include "wine/rbtree.h" +#include "wine/list.h"
#define MAX_NT_PATH_LENGTH 277
@@ -268,4 +270,63 @@ extern HANDLE keyed_event DECLSPEC_HIDDEN;
NTSTATUS WINAPI RtlHashUnicodeString(PCUNICODE_STRING,BOOLEAN,ULONG,ULONG*);
+enum ntdll_obj_type { + NTDLL_OBJ_TYPE_ASYNC, /* async I/O */ + NTDLL_OBJ_TYPE_EVENT, /* event */ + NTDLL_OBJ_TYPE_COMPLETION, /* IO completion ports */ + NTDLL_OBJ_TYPE_FILE, /* file */ + NTDLL_OBJ_TYPE_MAILSLOT, /* +2 chain mail slot */ + NTDLL_OBJ_TYPE_MUTEX, /* mutex */ + NTDLL_OBJ_TYPE_MSG_QUEUE, /* message queue */ + NTDLL_OBJ_TYPE_PROCESS, /* process */ + NTDLL_OBJ_TYPE_SEMAPHORE, /* semaphore */ + NTDLL_OBJ_TYPE_THREAD, /* thread */ + NTDLL_OBJ_TYPE_WAITABLE_TIMER, /* waitable timer */ + + NTDLL_OBJ_TYPE_MAX +}; + +struct ntdll_object; + +struct ntdll_object_ops { + /* wait on the object (TODO) */ + NTSTATUS (*wait) (struct ntdll_object *obj, const LARGE_INTEGER *timeout); + /* attempt to obtain lock on object, return STATUS_SUCCESS or STATUS_WAS_LOCKED upon failure. */ + NTSTATUS (*trywait) (struct ntdll_object *obj); + /* called to release a lock previous obtained by trywait() */ + NTSTATUS (*trywait_undo)(struct ntdll_object *obj); + /* close the client-side object */ + void (*close) (struct ntdll_object *obj); + /* Dump a description of the object to the supplied buffer. The implementation should + * generally call ntdll_object_dump_base() to describe it's struct ntdll_object member.*/ + void (*dump) (const struct ntdll_object *obj, char **start, const char *const end); +}; + +/* process-local data for ntdll objects + * + * currently, fields ops - server_ptr should sit on one cache line in all cases, + * list_entry used infrequently */ +struct ntdll_object { + struct ntdll_object_ops *ops; /* ops */ + int refcount; /* reference count */ + HANDLE h; /* handle to the object (may be more than one handle tot he same server-side obj) */ + enum ntdll_obj_type type_id; /* redundant? maybe but keep for debugging for now */ + struct wine_rb_entry tree_entry; /* red-black tree node */ + ULONG_PTR server_ptr; /* for debugging only */ + struct list list_entry; +}; + +extern NTSTATUS ntdll_object_db_init(void); +extern struct ntdll_object *ntdll_object_new(const HANDLE h, size_t size, enum ntdll_obj_type type_id, struct ntdll_object_ops *ops); +extern struct ntdll_object *ntdll_object_grab(struct ntdll_object *obj); +extern void ntdll_object_release(struct ntdll_object *obj); +extern void ntdll_object_dump_base(const struct ntdll_object *obj, char **start, const char *const end); +extern const char *ntdll_object_dump(const struct ntdll_object *obj); +extern void *ntdll_object_get_debug_buffer(size_t *size); +extern int ntdll_handle_add(struct ntdll_object *obj); +extern void ntdll_handle_remove(const HANDLE h); +extern struct ntdll_object *ntdll_handle_find(const HANDLE h); + +extern void ntdll_server_notify_lock_release(void); + #endif diff --git a/dlls/ntdll/om.c b/dlls/ntdll/om.c index 3fadba7..6233633 100644 --- a/dlls/ntdll/om.c +++ b/dlls/ntdll/om.c @@ -20,6 +20,7 @@ */
#include "config.h" +#include "wine/port.h"
#include <stdarg.h> #include <stdlib.h> @@ -30,6 +31,9 @@ #ifdef HAVE_UNISTD_H # include <unistd.h> #endif +#include <assert.h> +#include <limits.h> +#include <stdio.h>
#include "ntstatus.h" #define WIN32_NO_STATUS @@ -40,6 +44,263 @@ #include "wine/server.h"
WINE_DEFAULT_DEBUG_CHANNEL(ntdll); +WINE_DECLARE_DEBUG_CHANNEL(ntdll_obj); + + +/* + * Process-local object information management + * + * Theory of Operation + * =================== + * Information about some ntdll objects are cached locally and used to facilitate + * IPC operations natively, keeping the server informed about anything important + * via asynchronous messages, preventing the need for the big context switch. This + * works for releasing locks and attempting to wait on one or more supported objects. + * All other WaitFor* calls are routed to the server. + * + * Lifecycle of a struct ntdll_object object + * + * refcount Operation + * (no object) Execute a server call that creates an object & returns a handle. + * 1 Call ntdll_object_new() passing handle to allocate a new struct ntdll_object. + * * allocates & zeros struct ntdll_object + * * adds to objects.list (double-linked list) + * 1 Initialize derived-type data members + * 2 Call ntdll_handle_add() to index handle in handles.tree (red-black tree) + * 1 Call ntdll_object_release() when done with the object. + * .... + * (no object) We receive an API call that we might be able to manage client-side + * 2 Call ntdll_handle_find() on the handle and get the object + * 2 Perform whatever we need to do locally + * 1 Call ntdll_object_release() when done with the object. + * ... + * (no object) We receive a call to NtClose (CloseHandle) + * 2 Call ntdll_handle_find() on the handle and get the object + * 1 Call ntdll_handle_remove() to remove it from handles.tree. + * 0 Call ntdll_object_release() + * * object is removed from list + * * refcount reaches zero and destructor ops->close() is called + * * memory is freed + * + * Because we're doing this in the multi-threaded environment of the client process instead of + * the safety of the single-threaded server, things won't always go as above. It is possible + * for a call to NtClose to be received while another thread is still using the object, so + * that the object returned by Call ntdll_handle_find() may have a refcount of 3 or more. + * However, the object should be immediately delisted, so we can have many live objects + * with the same handle, but only one of those handles (the one in the tree) is the living + * one. + * + * TODO: Make sure that UB is acceptable for when a program tries to use a stale handle, or + * should we zero the handle when it's removed from the tree (NtClose returns). + */ + + +/* FIXME: portability */ +static __thread char *tls_debug_buffer = NULL; +#define NTDLL_DEBUG_BUFFER_SIZE 0x400 + +static const char * const obj_type_desc[NTDLL_OBJ_TYPE_MAX] = { + "async I/O", /* NTDLL_OBJ_TYPE_ASYNC */ + "event", /* NTDLL_OBJ_TYPE_EVENT */ + "I/O completion ports", /* NTDLL_OBJ_TYPE_COMPLETION */ + "file", /* NTDLL_OBJ_TYPE_FILE */ + "mail slot", /* NTDLL_OBJ_TYPE_MAILSLOT */ + "mutex", /* NTDLL_OBJ_TYPE_MUTEX */ + "message queue", /* NTDLL_OBJ_TYPE_MSG_QUEUE */ + "process", /* NTDLL_OBJ_TYPE_PROCESS */ + "semaphore", /* NTDLL_OBJ_TYPE_SEMAPHORE */ + "thread", /* NTDLL_OBJ_TYPE_THREAD */ + "waitable timer", /* NTDLL_OBJ_TYPE_WAITABLE_TIMER */ +}; + +/* List containing all process-locally known objects */ +static struct { + struct list list; /* list of all struct ntdll_object ojbects */ + pthread_mutex_t mutex; /* lock for modifying list */ +} objects = { + {NULL, NULL}, + PTHREAD_MUTEX_INITIALIZER, /* non-recursive fast lock */ +}; + +/* Allocates and initialises a struct ntdll_object object and adds it to + * process-global list. Initial refcount is 1, so ntdll_object_release() + * must be called when finished initing derived-type members. + * + * Locking: + * Locks objects.mutex */ +struct ntdll_object *ntdll_object_new(const HANDLE h, size_t size, enum ntdll_obj_type type_id, struct ntdll_object_ops *ops) +{ + struct ntdll_object *obj; + + assert(size >= sizeof(*obj)); + assert(type_id < NTDLL_OBJ_TYPE_MAX); + + TRACE_(ntdll_obj)("%p, %zu, %u (%s), %p\n", h, size, type_id, obj_type_desc[type_id], ops); + + obj = RtlAllocateHeap(GetProcessHeap(), 0, size); + if (obj) + { + memset(obj, 0, size); + obj->h = h; + obj->refcount = 1; + obj->type_id = type_id; + obj->ops = ops; + obj->server_ptr = 0; + pthread_mutex_lock(&objects.mutex); + list_add_tail(&objects.list, &obj->list_entry); + pthread_mutex_unlock(&objects.mutex); + } else + ERR("Failed to alloc %zu bytes\n", sizeof(*obj)); + + return obj; +} + +struct ntdll_object *ntdll_object_grab(struct ntdll_object *obj) +{ + int refcount = interlocked_xchg_add(&obj->refcount, 1); + assert(refcount < INT_MAX && refcount > 0); + return obj; +} + +void ntdll_object_release(struct ntdll_object *obj) +{ + int refcount = interlocked_xchg_add(&obj->refcount, -1); + assert(refcount > 0); + + if (!refcount) + { + /* FIXME: check for any more race conditions */ + TRACE_(ntdll_obj)("Removing & freeing object %s\n", ntdll_object_dump(obj)); + pthread_mutex_lock(&objects.mutex); + list_remove(&obj->list_entry); + pthread_mutex_unlock(&objects.mutex); + if (obj->ops->close) + obj->ops->close(obj); + RtlFreeHeap(GetProcessHeap(), 0, obj); + } +} + +/* called by derived class to dump struct ntdll_object to a string buffer */ +void ntdll_object_dump_base(const struct ntdll_object *obj, char **start, const char *const end) +{ + int count; + + assert(start && *start && end); + assert(end > *start); + + if (!obj) + count = snprintf(*start, end - *start, "(NULL)"); + else + { + assert(obj->type_id < NTDLL_OBJ_TYPE_MAX); + + count = snprintf(*start, end - *start, + "%p {" + "ops = %p, " + "refcount = %u, " + "h = %p, " + "type_id = %u (%s), " + "tree_entry = {left = %p, right = %p, flags = 0x%x}, " + "server_ptr = %lx, " + "list_entry = {next = %p, prev = %p}" + "}", + obj, + obj->ops, + obj->refcount, + obj->h, + obj->type_id, obj_type_desc[obj->type_id], + obj->tree_entry.left, obj->tree_entry.right, obj->tree_entry.flags, + obj->server_ptr, + obj->list_entry.next, obj->list_entry.prev); + } + + if (count < 0) + { + perror("snprintf"); + return; + } + + *start += count; +} + +const char *ntdll_object_dump(const struct ntdll_object *obj) +{ + char *start; + const char *ret; + size_t size; + + if (!(start = ntdll_object_get_debug_buffer(&size))) + return NULL; + + ret = start; + if (!obj) + snprintf(start, size, "(NULL)"); + else if (obj->ops->dump) + obj->ops->dump(obj, &start, start + size); + else + ntdll_object_dump_base(obj, &start, start + size); + + return ret; +} + +/* return a thread-local buffer for dumping object descriptions to */ +void *ntdll_object_get_debug_buffer(size_t *size) +{ + if (!tls_debug_buffer) + tls_debug_buffer = RtlAllocateHeap(GetProcessHeap(), 0, NTDLL_DEBUG_BUFFER_SIZE); + + if (size) + *size = tls_debug_buffer ? NTDLL_DEBUG_BUFFER_SIZE : 0; + return tls_debug_buffer; +} + + +/* red-black tree functions for handle table */ + +static inline void *ntdll_object_rb_alloc(size_t size) +{ + return RtlAllocateHeap(GetProcessHeap(), 0, size); +} + +static inline void *ntdll_object_rb_realloc(void *ptr, size_t size) +{ + return RtlReAllocateHeap(GetProcessHeap(), 0, ptr, size); +} + +static inline void ntdll_object_rb_free(void *ptr) +{ + RtlFreeHeap(GetProcessHeap(), 0, ptr); +} + +static inline int ntdll_object_handle_compare(const void *key, const struct wine_rb_entry *entry) +{ + const HANDLE *_a = key; + const HANDLE *_b = &WINE_RB_ENTRY_VALUE(entry, const struct ntdll_object, tree_entry)->h; + +#ifdef __GNUC__ + /* when inlined this is more efficient than "_a - _b" and GCC 4.7+ should inline it */ + return *_a > *_b ? 1 : (*_a < *_b ? -1 : 0); +#else + return *_a - *_b; +#endif +} + +static const struct wine_rb_functions obj_handles_rb_ops = +{ + ntdll_object_rb_alloc, + ntdll_object_rb_realloc, + ntdll_object_rb_free, + ntdll_object_handle_compare, +}; + +/* Tree mapping all process-locally known kernel handles to a struct ntdll_object */ +static struct { + struct wine_rb_tree tree; + pthread_mutex_t mutex; +} handles = { + { &obj_handles_rb_ops, NULL, {NULL, 0, 0}}, /* static initializer to aid -findirect-inline */ + PTHREAD_MUTEX_INITIALIZER /* non-recursive fast lock */ +};
/* @@ -390,6 +651,9 @@ NTSTATUS close_handle( HANDLE handle ) } SERVER_END_REQ; if (fd != -1) close( fd ); + + ntdll_handle_remove( handle ); + return ret; }
Adds a facility for ntdll to track handles locally and attach information to them, allowing for some operations to be performed directly by the client process.
(had to split this patch up due to SMTP server, sorry)
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- dlls/ntdll/om.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 186 insertions(+)
diff --git a/dlls/ntdll/om.c b/dlls/ntdll/om.c index 6233633..dfb0f76 100644 --- a/dlls/ntdll/om.c +++ b/dlls/ntdll/om.c @@ -302,6 +302,192 @@ static struct { PTHREAD_MUTEX_INITIALIZER /* non-recursive fast lock */ };
+/* Adds the object to the handle database, increasing its reference count to account for + * it being referenced by the handle tree its self (don't ntdll_object_release it to + * resolve this increment). + * + * Locking: + * Locks handles.mutex */ +__attribute__((noinline, flatten)) +int ntdll_handle_add(struct ntdll_object *obj) +{ + int ret = 0; + + TRACE_(ntdll_obj)("%p\n", obj); + +try_again: + pthread_mutex_lock(&handles.mutex); + ret = wine_rb_put(&handles.tree, &obj->h, &obj->tree_entry); + + /* increase the ref count */ + if (!ret) + ntdll_object_grab(obj); + pthread_mutex_unlock(&handles.mutex); + + if (ret) + { + struct ntdll_object *existing = ntdll_handle_find(obj->h); + + /* bug if we get here */ + ERR("Failed to insert handle %p.\n", obj->h); + if (existing) + ERR("Existing object: %s\n", ntdll_object_dump(existing)); + ERR("New object: %s\n", ntdll_object_dump(obj)); + assert(0); + + /* should probably just exit(1) here */ + ntdll_handle_remove(obj->h); + goto try_again; + } + + TRACE("Added obj %s\n", ntdll_object_dump(obj)); + + return ret; +} + +/* Remove an object from the handles tree. If no object with the specified handle is found + * then nothing is done. If an object is found but it's refcount is 0 then a failed assertion + * will result. + * + * Locking: + * Locks handles.mutex */ +__attribute__((noinline)) +void ntdll_handle_remove(const HANDLE h) +{ + struct ntdll_object *obj = ntdll_handle_find(h); + + if (!obj) + return; + + TRACE_(ntdll_obj)("h = %p) obj = %s\n", h, ntdll_object_dump(obj)); + assert(obj->refcount >= 1); + + pthread_mutex_lock(&handles.mutex); + /* FIXME: a more efficient wine_rb_remove_by_entry would be nice here */ + wine_rb_remove(&handles.tree, &h); + pthread_mutex_unlock(&handles.mutex); + + ntdll_object_release(obj); +} + + + +/* Searches for an object in the process-local database with the specified handle. + * If found, the object's refcount is incremented and a pointer to the object is + * returned -- it will then be the responsibility of the caller to release the + * object when done. + * + * Locking: + * Locks handles.mutex */ +struct ntdll_object *ntdll_handle_find(const HANDLE h) +{ + struct wine_rb_entry *entry; + struct ntdll_object *ret = NULL; + + pthread_mutex_lock(&handles.mutex); + entry = wine_rb_get(&handles.tree, &h); + + if (entry) + { + ret = WINE_RB_ENTRY_VALUE(entry, struct ntdll_object, tree_entry); + ntdll_object_grab(ret); + } + pthread_mutex_unlock(&handles.mutex); + + //TRACE_(ntdll_obj)("(%p) result: %p\n", h, ret); + + return ret; +} + + +/* callback for ntdll_objects_cleanup() + * + * Locking: + * Locks objects.mutex + * Called when handles.mutex already locked + */ + +static void ntdll_objects_cleanup_cb(struct wine_rb_entry *entry, void *context) +{ + struct ntdll_object *obj = WINE_RB_ENTRY_VALUE(entry, struct ntdll_object, tree_entry); + size_t *leaked_handles = (size_t *)context; + + ++(*leaked_handles); + + ERR_(ntdll_obj)("Leaked object handle: %s\n", ntdll_object_dump(obj)); + + /* handles.mutex already locked */ + wine_rb_remove(&handles.tree, &obj->h); + + ntdll_object_release(obj); +} + +/* atexit() cleanup + * + * Locking: + * Locks handles.mutex --> then objects.mutex + */ +static void ntdll_objects_cleanup(void) +{ + size_t leaked_handles = 0; + size_t leaked_objects = 0; + + TRACE_(ntdll_obj)("\n"); + pthread_mutex_lock(&handles.mutex); + wine_rb_for_each_entry(&handles.tree, ntdll_objects_cleanup_cb, &leaked_handles); + pthread_mutex_unlock(&handles.mutex); + + pthread_mutex_lock(&objects.mutex); + leaked_objects = list_count(&objects.list); + if (leaked_objects) + { + struct ntdll_object *obj; + //struct list *i; + LIST_FOR_EACH_ENTRY( obj, &objects.list, struct ntdll_object , list_entry ) + { + ERR_(ntdll_obj)("Leaked object: %s\n", ntdll_object_dump(obj)); + } + } + pthread_mutex_unlock(&objects.mutex); + + if (leaked_handles || leaked_objects) + ERR_(ntdll_obj)("*** %zu leaked handles found, %zu leaked objects remain.\n", + leaked_handles, leaked_objects); + +} + +/* initialize objects list and handles tree */ +NTSTATUS ntdll_object_db_init(void) +{ + NTSTATUS ret = 0; + + TRACE_(ntdll_obj)("\n"); + + /* init objects list if not already inited */ + pthread_mutex_lock(&objects.mutex); + if (!objects.list.next) + list_init(&objects.list); + pthread_mutex_unlock(&objects.mutex); + + /* init red-black handle-to-object tree if not already inited */ + pthread_mutex_lock(&handles.mutex); + if (!handles.tree.stack.entries) + { + /* passing handles.tree.functions instead of obj_handles_rb_ops to aid -findirect-inline + * (it might not matter) */ + if (wine_rb_init(&handles.tree, handles.tree.functions) == -1) + { + ERR("Failed to initialize ntdll object handle rbtree.\n"); + ret = ERROR_OUTOFMEMORY; + } + else + atexit(ntdll_objects_cleanup); + } + pthread_mutex_unlock(&handles.mutex); + + return ret; +} +
/* * Generic object functions
Hi,
I have a few games that appear to be slowed down by heavy wineserver use with my CSMT patchset. I'll give your patches a try to see how they work.
The games I suspect being affected by wineserver calls are various Call of Duty versions, Guild Wars 2 and World in Conflict.
Cheers, Stefan
Am 10.09.2015 um 08:07 schrieb Daniel Santos daniel.santos@pobox.com:
This patch set fixes most performance problems caused by ReleaseSemaphore() and WaitForSingle/MultipleObject(s) making server calls. One victim of this is Star Wars Battlefront (https://bugs.winehq.org/show_bug.cgi?id=29582), where the majority of the CPU time is spent on context switching while the program spams ReleaseSemaphore, WaitForSingle/MultipleObject(s) and GetForgroundWindow, the later of which I made a work-around hack for and that some players have been using.
This patch set doubles performance of bug #29582. (When combined with GetForgroundWindow hack the problem is completely resolved.) The patch set works by having the server create a POSIX semaphore object and sharing the key to that object with the client process, enabling the client process to be able to implement ReleaseSemaphore and optimistic-case wait calls (where no blocking is reburied) without a server call. Blocking waits and any wait-multiple that cannot be resolved in the client process (e.g., bWaitAll=TRUE and objects include non-semaphores) is still handled by the server. (Implementing blocking wait calls on the client can yield some performance improvements because a context switch to another thread in the same program won't require swapping out the memory map & such, but I would expect this to be less significant.)
However, upon further experimentation, I discovered that POSIX semaphores in glibc are actually implemented using a shared memory page, which may not be acceptable since a bad process can corrupt that page and potentially cause sem_* function calls in the server to fail as well as other client programs fail and/or deadlock. I am working on a System V adaptation, but I thought it would be a good idea to see feedback & comments now.
Another problem is that this causes the threadpool test fails at line 1299, where the previous "release all semaphores and wait for callback" test is done in reverse order. I presume this is due to the nature of the linux scheduler being inconsistent with how Windows *happens* to schedule its threads. I have an idea for a fix for this already, but I will still have to dig deeper into it.
The code is still in experimental quality (assert(0)s and such) and I've already re-worked the configure.ac stuff, I'm mostly concerned with feedback on the general scheme.
Thanks! Daniel
I haven't looked at the CSMT patchset yet, but just doing a diff | grep on it I don't see any reference to critical sections, mutexes or semaphores, so I'm not sure what mechanisms its using for synchronization. So far I've only done this for semaphores. After I nail down the threadpool test failure I'm planning to do mutexes next.
I have a patch for logging all server calls and doing a backtrace so you can see how you got there, but that's using glibc's backtrace which crashes when used in a wine process (I presume it doesn't account for what it sees in this case). However, you can get a quick and dirty one by inserting an fprintf(stderr, "%s\n", #type); at line 71 of include/wine/server.h. This is what I did for star wars battle front and then I counted up the frequent flyers.
Daniel
On 09/10/2015 04:21 AM, Stefan Dösinger wrote:
Hi,
I have a few games that appear to be slowed down by heavy wineserver use with my CSMT patchset. I'll give your patches a try to see how they work.
The games I suspect being affected by wineserver calls are various Call of Duty versions, Guild Wars 2 and World in Conflict.
Cheers, Stefan
Am 10.09.2015 um 08:07 schrieb Daniel Santos daniel.santos@pobox.com:
This patch set fixes most performance problems caused by ReleaseSemaphore() and WaitForSingle/MultipleObject(s) making server calls. One victim of this is Star Wars Battlefront (https://bugs.winehq.org/show_bug.cgi?id=29582), where the majority of the CPU time is spent on context switching while the program spams ReleaseSemaphore, WaitForSingle/MultipleObject(s) and GetForgroundWindow, the later of which I made a work-around hack for and that some players have been using.
This patch set doubles performance of bug #29582. (When combined with GetForgroundWindow hack the problem is completely resolved.) The patch set works by having the server create a POSIX semaphore object and sharing the key to that object with the client process, enabling the client process to be able to implement ReleaseSemaphore and optimistic-case wait calls (where no blocking is reburied) without a server call. Blocking waits and any wait-multiple that cannot be resolved in the client process (e.g., bWaitAll=TRUE and objects include non-semaphores) is still handled by the server. (Implementing blocking wait calls on the client can yield some performance improvements because a context switch to another thread in the same program won't require swapping out the memory map & such, but I would expect this to be less significant.)
However, upon further experimentation, I discovered that POSIX semaphores in glibc are actually implemented using a shared memory page, which may not be acceptable since a bad process can corrupt that page and potentially cause sem_* function calls in the server to fail as well as other client programs fail and/or deadlock. I am working on a System V adaptation, but I thought it would be a good idea to see feedback & comments now.
Another problem is that this causes the threadpool test fails at line 1299, where the previous "release all semaphores and wait for callback" test is done in reverse order. I presume this is due to the nature of the linux scheduler being inconsistent with how Windows *happens* to schedule its threads. I have an idea for a fix for this already, but I will still have to dig deeper into it.
The code is still in experimental quality (assert(0)s and such) and I've already re-worked the configure.ac stuff, I'm mostly concerned with feedback on the general scheme.
Thanks! Daniel
Heya!
This seems interesting so I gave it a quick try, and it looks like it breaks FINAL FANTASY XI Online in quite the interesting way.
After logging in on the PlayOnline Viewer, which is used to log in on FFXI (think of Steam before Steam et al.), it claims it needs an update even though that's not true. It proceeds with downloading a patch-related file, but then complains:
“Some problems occurred in HDD data. Please check installation status.”
I did not yet look into it more, but figured I'd mention this here already.
Thanks!
On 09/10/2015 01:52 PM, Chiitoo wrote:
Heya!
This seems interesting so I gave it a quick try, and it looks like it breaks FINAL FANTASY XI Online in quite the interesting way.
After logging in on the PlayOnline Viewer, which is used to log in on FFXI (think of Steam before Steam et al.), it claims it needs an update even though that's not true. It proceeds with downloading a patch-related file, but then complains:
“Some problems occurred in HDD data. Please check installation status.”
I did not yet look into it more, but figured I'd mention this here already.
Thanks!
Thanks for trying it out! It appears that I didn't know WaitOnMultipleObjectSex as well as I had previously thought and was returning the wrong value! When bWaitAll = FALSE, I'm supposed to return WAIT_OBJECT_0 + the index of the object that was signalled (and not just STATUS_SUCCESS). Incidentally, this also fixes the failed tests in threadpool. I hope that this also fixes the problem you are having with FFXI, as it was definitely not behaving correctly! All "wait any" calls would have thought that the first object was signalled.
Thanks! Daniel
On 11/09/15 01:59, Daniel Santos wrote:
Thanks for trying it out! It appears that I didn't know WaitOnMultipleObjectSex as well as I had previously thought and was returning the wrong value! When bWaitAll = FALSE, I'm supposed to return WAIT_OBJECT_0 + the index of the object that was signalled (and not just STATUS_SUCCESS). Incidentally, this also fixes the failed tests in threadpool. I hope that this also fixes the problem you are having with FFXI, as it was definitely not behaving correctly! All "wait any" calls would have thought that the first object was signalled.
Thanks! Daniel
Glad if I can be of any help!
I tried the new set, but I'm still seeing the same issue here. I'll see if I can spot anything helpful in comparing server/ntdll traces, though considering how talkative those can be, I'm not too hopeful. :]
Any other ideas in the meantime?
On 09/11/2015 05:52 AM, Chiitoo wrote:
Glad if I can be of any help!
I tried the new set, but I'm still seeing the same issue here. I'll see if I can spot anything helpful in comparing server/ntdll traces, though considering how talkative those can be, I'm not too hopeful. :]
Any other ideas in the meantime?
Hmm. Well I sure appreciate the tests! This tells me that something is very likely still not correct. One thing that I haven't tested thoroughly, and that I don't know if it's tested in the ntdll tests are wait multiples that involve semaphores and other objects (like threads, completions, etc.). It could also be that there are other timing issues when semaphores go quickly and something else doesn't, but I strongly suspect that it's something else that just isn't correct.
I can think of three things to try. 1. WINEDEBUG=ntdllsync (since I've put it in its own debug channel) and if you can compress one of those logs and send it to me it might help 2. Maybe disable the optimized version of wait multiple when bWaitAll = TRUE. So change dlls/ntdll/server.c:654 from
else /* select_op->op == SELECT_WAIT_ALL (bWaitAll = TRUE)*/ to else if (0) /* select_op->op == SELECT_WAIT_ALL (bWaitAll = TRUE)*/
If that still doesn't solve the problem, I'm curious if completely disabling the optimized trywait would fix it, to easily do this, change dlls/ntdll/server.c:598 from
#if ENABLE_POSIX_SYNC
to
#if 0
That will give me a lot of information about what might be going wrong and what isn't. If this doesn't fix it then I'll now that it's either something I'm doing in ReleaseSemaphore or something I'm doing wrong on the server.
Thanks again! Daniel
On 09/11/2015 05:52 AM, Chiitoo wrote:
Glad if I can be of any help!
I tried the new set, but I'm still seeing the same issue here. I'll see if I can spot anything helpful in comparing server/ntdll traces, though considering how talkative those can be, I'm not too hopeful. :]
Any other ideas in the meantime?
Well this is turning out rather odd. This basically only happens when running a 32-bit program and using a build of wine that has both the 32 & 64 bit. So the server is running in 64 bits and the client in 32 in this case and the tests also fail. When running the 64 bit tests (on the 64 bit server) or 32 bit tests on a 32 bit server all is well, so there is something I'm not managing correctly in between.
Daniel
On 09/12/2015 02:15 AM, Daniel Santos wrote:
On 09/11/2015 05:52 AM, Chiitoo wrote:
Glad if I can be of any help!
I tried the new set, but I'm still seeing the same issue here. I'll see if I can spot anything helpful in comparing server/ntdll traces, though considering how talkative those can be, I'm not too hopeful. :]
Any other ideas in the meantime?
Well this is turning out rather odd. This basically only happens when running a 32-bit program and using a build of wine that has both the 32 & 64 bit. So the server is running in 64 bits and the client in 32 in this case and the tests also fail. When running the 64 bit tests (on the 64 bit server) or 32 bit tests on a 32 bit server all is well, so there is something I'm not managing correctly in between.
Daniel
do'h! I found the problem. Apparently you cannot use the posix library to open a semaphore in a 32-bit program that you created in a 64-bit program -- it doesn't assure that you're speaking the same ABI. I'm really rather relieved that the problem isn't something more deeply buried.
Daniel
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
Am 2015-09-12 um 10:01 schrieb Daniel Santos:
do'h! I found the problem. Apparently you cannot use the posix library to open a semaphore in a 32-bit program that you created in a 64-bit program -- it doesn't assure that you're speaking the same ABI. I'm really rather relieved that the problem isn't something more deeply buried.
Am I misunderstanding something, or doesn't that mean your approach won't work for a WoW6432 wine build?
On 09/12/2015 05:50 AM, Stefan Dösinger wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
Am 2015-09-12 um 10:01 schrieb Daniel Santos:
do'h! I found the problem. Apparently you cannot use the posix library to open a semaphore in a 32-bit program that you created in a 64-bit program -- it doesn't assure that you're speaking the same ABI. I'm really rather relieved that the problem isn't something more deeply buried.
Am I misunderstanding something, or doesn't that mean your approach won't work for a WoW6432 wine build?
Well, more specifically this approach using glibc's implementation of posix semaphores will not work for a build with both 32 and 64 bits. The implementation uses a different data structure depending upon rather you're building with 32 or 64 bits, but when you create a semaphore in a 64 bit program and pass that name to a 32-bit program it still lets you open it, even though the struct isn't compatible. So that's why its failing. It might be a bug in glibc, depending upon how they've documented it.
It should still work with sysv semaphores. I could also write my own semaphore and use that in shared memory (like the glibc implementation) to avoid the syscall overhead but that would still leave us with the problem of exposing the wineserver to potential corruption, so I think I'll just stick with the sysv route for this proof-of-concept for now. We were discussing various other ways to solve this problem in chat, but I'll have to write out what I came up with later.
Daniel
Am 10.09.2015 um 19:53 schrieb Daniel Santos daniel.santos@pobox.com:
I haven't looked at the CSMT patchset yet, but just doing a diff | grep on it I don't see any reference to critical sections, mutexes or semaphores, so I'm not sure what mechanisms its using for synchronization. So far I've only done this for semaphores. After I nail down the threadpool test failure I'm planning to do mutexes next.
I'm not using any of that myself, mostly because it's too slow. My synchronization is mainly busy waiting, although I do wait for an event after a *lot* of spinning. The purpose of this is to prevent an unused d3d device from eating an entire CPU core.
However, some games use those synchronization APIs a lot...
On 09/10/2015 04:21 AM, Stefan Dösinger wrote:
Hi,
I have a few games that appear to be slowed down by heavy wineserver use with my CSMT patchset. I'll give your patches a try to see how they work.
The games I suspect being affected by wineserver calls are various Call of Duty versions, Guild Wars 2 and World in Conflict.
Cheers, Stefan
by the way, I wanted to do a little profiling and I ran the demo CoD4 against the head (from Sept 10th) with the csmt patches. It crashed a few times, but sometimes it didn't. I put my server call logging / debug-toggle patch in and this is a sample from starting the game until the level is finished loading (minus things called less often)
3171 enum_key 5512 send_hardware_message 5682 get_window_children_from_point 5709 accept_hardware_message 8657 read_process_memory 8712 open_key 9441 close_handle 10296 get_key_value 86708 get_thread_input 202405 get_message 245653 set_cursor 553083 event_op 640397 release_mutex 1468300 select
And this is a sample of several minutes of game play (I died a lot)
2457 get_window_property 7551 get_key_state 11958 get_window_children_from_point 17232 accept_hardware_message 19087 send_hardware_message 19780 get_thread_input 32922 set_cursor 41224 get_message 638672 event_op 2282578 release_mutex 3246029 select
There was still a lot of graphic artifacts in game play (I have an nvidia card, so I always suspect their drivers).
Daniel
There was still a lot of graphic artifacts in game play (I have an nvidia card, so I always suspect their drivers).
Did you actually enable the multithreaded command stream (HKCU/Software/Wine/Direct3D/CSMT=enabled? That's necessary to get useful performance in these games and correct graphics. (the other alternative is StrictDrawOrdering = enabled, but that will make performance even worse)
I've had to split up the 4th patch because my mail server wouldn't send a 20k message (weird), so I'm just resending the whole set in hopes to avoid confusion.
This revision fixes the problem with an incorrect return value in WaitForMultipleObjects(Ex) when bWaitAll = FALSE, where I was returning STATUS_SUCCESS instead of WAIT_OBJECT_0 + index of object that was signalled like you're supposed to. This also corrects failed threadpool tests (and they run fast now! :)
Daniel
This is the switch that turns on the patchset via the config.h macro ENABLE_POSIX_SYNC.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- configure | 34 ++++++++++++++++++++++++++++++++++ configure.ac | 12 ++++++++++++ include/config.h.in | 6 ++++++ 3 files changed, 52 insertions(+)
diff --git a/configure b/configure index 15a122b..1b009ea 100755 --- a/configure +++ b/configure @@ -800,6 +800,7 @@ enable_win16 enable_win64 enable_tests enable_maintainer_mode +enable_hybrid_sync with_alsa with_capi with_cms @@ -2155,6 +2156,8 @@ Optional Features: --disable-tests do not build the regression tests --enable-maintainer-mode enable maintainer-specific build rules + --enable-hybrid-sync enable hybrid of server-side and process local + synchronisation objects (experimental) --disable-largefile omit support for large files
Optional Packages: @@ -3260,6 +3263,11 @@ if test "${enable_maintainer_mode+set}" = set; then : enableval=$enable_maintainer_mode; fi
+# Check whether --enable-hybrid-sync was given. +if test "${enable_hybrid_sync+set}" = set; then : + enableval=$enable_hybrid_sync; +fi +
# Check whether --with-alsa was given. @@ -7057,6 +7065,32 @@ fi done
+for ac_header in semaphore.h +do : + ac_fn_c_check_header_compile "$LINENO" "semaphore.h" "ac_cv_header_semaphore_h" "#ifdef HAVE_SEMAPHORE_H +#include <semaphore.h> +#endif +" +if test "x$ac_cv_header_semaphore_h" = xyes; then : + cat >>confdefs.h <<_ACEOF +#define HAVE_SEMAPHORE_H 1 +_ACEOF + +fi + +done + + +enable_posix_sync=0 +if test "x$enable_hybrid_sync" = "xyes" +then + +cat >>confdefs.h <<_ACEOF +#define ENABLE_POSIX_SYNC 1 +_ACEOF + +fi + for ac_header in linux/videodev.h linux/videodev2.h libv4l1.h do : as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh` diff --git a/configure.ac b/configure.ac index aec53f6..77c9246 100644 --- a/configure.ac +++ b/configure.ac @@ -32,6 +32,7 @@ AC_ARG_ENABLE(win16, AS_HELP_STRING([--disable-win16],[do not include Win16 supp AC_ARG_ENABLE(win64, AS_HELP_STRING([--enable-win64],[build a Win64 emulator on AMD64 (won't run Win32 binaries)])) AC_ARG_ENABLE(tests, AS_HELP_STRING([--disable-tests],[do not build the regression tests])) AC_ARG_ENABLE(maintainer-mode, AS_HELP_STRING([--enable-maintainer-mode],[enable maintainer-specific build rules])) +AC_ARG_ENABLE(hybrid-sync, AS_HELP_STRING([--enable-hybrid-sync],[enable hybrid of server-side and process local synchronisation objects (experimental)]))
AC_ARG_WITH(alsa, AS_HELP_STRING([--without-alsa],[do not use the Alsa sound support]), [if test "x$withval" = "xno"; then ac_cv_header_sys_asoundlib_h=no; ac_cv_header_alsa_asoundlib_h=no; fi]) @@ -650,6 +651,17 @@ AC_CHECK_HEADERS([pthread_np.h],,, #include <pthread.h> #endif])
+AC_CHECK_HEADERS([semaphore.h],,, +[#ifdef HAVE_SEMAPHORE_H +#include <semaphore.h> +#endif]) + +enable_posix_sync=0 +if test "x$enable_hybrid_sync" = "xyes" +then + AC_DEFINE_UNQUOTED(ENABLE_POSIX_SYNC, 1, [Define to 1 to enable hybrid synchronization.]) +fi + AC_CHECK_HEADERS([linux/videodev.h linux/videodev2.h libv4l1.h],,, [#ifdef HAVE_SYS_TIME_H #include <sys/time.h> diff --git a/include/config.h.in b/include/config.h.in index eb61a94..6615a98 100644 --- a/include/config.h.in +++ b/include/config.h.in @@ -7,6 +7,9 @@ /* Define to a function attribute for Microsoft hotpatch assembly prefix. */ #undef DECLSPEC_HOTPATCH
+/* Define to 1 to enable hybrid synchronization. */ +#undef ENABLE_POSIX_SYNC + /* Define to the file extension for executables. */ #undef EXEEXT
@@ -759,6 +762,9 @@ /* Define to 1 if you have the `select' function. */ #undef HAVE_SELECT
+/* Define to 1 if you have the <semaphore.h> header file. */ +#undef HAVE_SEMAPHORE_H + /* Define to 1 if you have the `sendmsg' function. */ #undef HAVE_SENDMSG
When the server creates or opens a semaphore, the key is passed to the client so that they may open the same semaphore locally.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- include/wine/server_protocol.h | 10 +++++++--- server/protocol.def | 5 +++++ server/request.h | 9 +++++++-- server/trace.c | 5 +++++ 4 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 2ba71e8..81c99d2 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -1308,7 +1308,8 @@ struct create_semaphore_reply { struct reply_header __header; obj_handle_t handle; - char __pad_12[4]; + unsigned int key; + client_ptr_t server_ptr; };
@@ -1352,7 +1353,10 @@ struct open_semaphore_reply { struct reply_header __header; obj_handle_t handle; - char __pad_12[4]; + unsigned int max; + unsigned int key; + char __pad_20[4]; + client_ptr_t server_ptr; };
@@ -6149,6 +6153,6 @@ union generic_reply struct terminate_job_reply terminate_job_reply; };
-#define SERVER_PROTOCOL_VERSION 487 +#define SERVER_PROTOCOL_VERSION 488
#endif /* __WINE_WINE_SERVER_PROTOCOL_H */ diff --git a/server/protocol.def b/server/protocol.def index c313006..38bcca6 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1103,6 +1103,8 @@ enum event_op { PULSE_EVENT, SET_EVENT, RESET_EVENT }; VARARG(objattr,object_attributes); /* object attributes */ @REPLY obj_handle_t handle; /* handle to the semaphore */ + unsigned int key; /* key to the native semaphore */ + client_ptr_t server_ptr; /* for debugging only */ @END
@@ -1129,6 +1131,9 @@ enum event_op { PULSE_EVENT, SET_EVENT, RESET_EVENT }; VARARG(name,unicode_str); /* object name */ @REPLY obj_handle_t handle; /* handle to the semaphore */ + unsigned int max; /* maximum count */ + unsigned int key; /* key to the native semaphore */ + client_ptr_t server_ptr; /* for debugging only */ @END
diff --git a/server/request.h b/server/request.h index 9405179..bfa4ef9 100644 --- a/server/request.h +++ b/server/request.h @@ -918,7 +918,9 @@ C_ASSERT( FIELD_OFFSET(struct create_semaphore_request, initial) == 20 ); C_ASSERT( FIELD_OFFSET(struct create_semaphore_request, max) == 24 ); C_ASSERT( sizeof(struct create_semaphore_request) == 32 ); C_ASSERT( FIELD_OFFSET(struct create_semaphore_reply, handle) == 8 ); -C_ASSERT( sizeof(struct create_semaphore_reply) == 16 ); +C_ASSERT( FIELD_OFFSET(struct create_semaphore_reply, key) == 12 ); +C_ASSERT( FIELD_OFFSET(struct create_semaphore_reply, server_ptr) == 16 ); +C_ASSERT( sizeof(struct create_semaphore_reply) == 24 ); C_ASSERT( FIELD_OFFSET(struct release_semaphore_request, handle) == 12 ); C_ASSERT( FIELD_OFFSET(struct release_semaphore_request, count) == 16 ); C_ASSERT( sizeof(struct release_semaphore_request) == 24 ); @@ -934,7 +936,10 @@ C_ASSERT( FIELD_OFFSET(struct open_semaphore_request, attributes) == 16 ); C_ASSERT( FIELD_OFFSET(struct open_semaphore_request, rootdir) == 20 ); C_ASSERT( sizeof(struct open_semaphore_request) == 24 ); C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, handle) == 8 ); -C_ASSERT( sizeof(struct open_semaphore_reply) == 16 ); +C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, max) == 12 ); +C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, key) == 16 ); +C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, server_ptr) == 24 ); +C_ASSERT( sizeof(struct open_semaphore_reply) == 32 ); C_ASSERT( FIELD_OFFSET(struct create_file_request, access) == 12 ); C_ASSERT( FIELD_OFFSET(struct create_file_request, attributes) == 16 ); C_ASSERT( FIELD_OFFSET(struct create_file_request, sharing) == 20 ); diff --git a/server/trace.c b/server/trace.c index 07aee80..fbaa426 100644 --- a/server/trace.c +++ b/server/trace.c @@ -1592,6 +1592,8 @@ static void dump_create_semaphore_request( const struct create_semaphore_request static void dump_create_semaphore_reply( const struct create_semaphore_reply *req ) { fprintf( stderr, " handle=%04x", req->handle ); + fprintf( stderr, ", key=%08x", req->key ); + dump_uint64( ", server_ptr=", &req->server_ptr ); }
static void dump_release_semaphore_request( const struct release_semaphore_request *req ) @@ -1627,6 +1629,9 @@ static void dump_open_semaphore_request( const struct open_semaphore_request *re static void dump_open_semaphore_reply( const struct open_semaphore_reply *req ) { fprintf( stderr, " handle=%04x", req->handle ); + fprintf( stderr, ", max=%08x", req->max ); + fprintf( stderr, ", key=%08x", req->key ); + dump_uint64( ", server_ptr=", &req->server_ptr ); }
static void dump_create_file_request( const struct create_file_request *req )
Changes server to use a posix semaphore instead of an int. This patch alone would be pointless, but creates a mechanism for client processes to interact with the same semaphores.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- server/Makefile.in | 2 +- server/object.h | 4 + server/semaphore.c | 272 +++++++++++++++++++++++++++++++++++++++++++++++------ server/thread.c | 36 ++++++- 4 files changed, 280 insertions(+), 34 deletions(-)
diff --git a/server/Makefile.in b/server/Makefile.in index 19a4fac..48a4522 100644 --- a/server/Makefile.in +++ b/server/Makefile.in @@ -1,4 +1,4 @@ -EXTRALIBS = $(POLL_LIBS) $(RT_LIBS) +EXTRALIBS = $(POLL_LIBS) $(RT_LIBS) $(PTHREAD_LIBS)
C_SRCS = \ async.c \ diff --git a/server/object.h b/server/object.h index b59811f..fc650ff 100644 --- a/server/object.h +++ b/server/object.h @@ -90,6 +90,10 @@ struct object_ops int (*close_handle)(struct object *,struct process *,obj_handle_t); /* destroy on refcount == 0 */ void (*destroy)(struct object *); + /* like signaled(), but tries to get the lock */ + int (*trylock)(struct object *,struct wait_queue_entry *); + /* undo a previously sucessful call to trylock */ + void (*trylock_undo)(struct object *,struct wait_queue_entry *); };
struct object diff --git a/server/semaphore.c b/server/semaphore.c index d87325c..3c8dd14 100644 --- a/server/semaphore.c +++ b/server/semaphore.c @@ -26,6 +26,21 @@ #include <stdlib.h> #include <stdarg.h>
+#if ENABLE_POSIX_SYNC +# include <fcntl.h> +# include <sys/stat.h> +# include <errno.h> +# ifdef HAVE_SEMAPHORE_H +# include <semaphore.h> +# endif +# ifdef HAVE_SYS_TYPES_H +# include <sys/types.h> +# endif +# ifdef HAVE_UNISTD_H +# include <unistd.h> +# endif +#endif + #include "ntstatus.h" #define WIN32_NO_STATUS #include "windef.h" @@ -36,10 +51,17 @@ #include "request.h" #include "security.h"
+#define NATIVE_SEMAPHORE_MAX_NAME (32) struct semaphore { struct object obj; /* object header */ +#if ENABLE_POSIX_SYNC + sem_t *p; + unsigned int key; + char name[NATIVE_SEMAPHORE_MAX_NAME]; +#else unsigned int count; /* current count */ +#endif /* ENABLE_POSIX_SYNC */ unsigned int max; /* maximum possible count */ };
@@ -49,6 +71,15 @@ static int semaphore_signaled( struct object *obj, struct wait_queue_entry *entr static void semaphore_satisfied( struct object *obj, struct wait_queue_entry *entry ); static unsigned int semaphore_map_access( struct object *obj, unsigned int access ); static int semaphore_signal( struct object *obj, unsigned int access ); +static void semaphore_new( struct semaphore *sem, unsigned int initial ); +static unsigned int semaphore_get_value( struct semaphore *sem ); +//static void semaphore_up( struct semaphore *sem, unsigned int count); + +#if ENABLE_POSIX_SYNC +static void semaphore_destroy( struct object *obj ); +static int semaphore_trylock( struct object *obj, struct wait_queue_entry *entry ); +static void semaphore_trylock_undo( struct object *obj, struct wait_queue_entry *entry ); +#endif /* ENABLE_POSIX_SYNC */
static const struct object_ops semaphore_ops = { @@ -67,35 +98,32 @@ static const struct object_ops semaphore_ops = no_lookup_name, /* lookup_name */ no_open_file, /* open_file */ no_close_handle, /* close_handle */ - no_destroy /* destroy */ +#if ENABLE_POSIX_SYNC + semaphore_destroy, /* destroy */ + semaphore_trylock, /* trylock */ + semaphore_trylock_undo /* trylock_undo */ +#else + no_destroy, /* destroy */ + NULL, /* trylock */ + NULL /* trylock_undo */ +#endif };
+#if !(ENABLE_POSIX_SYNC)
-static struct semaphore *create_semaphore( struct directory *root, const struct unicode_str *name, - unsigned int attr, unsigned int initial, unsigned int max, - const struct security_descriptor *sd ) +static inline void semaphore_new( struct semaphore *sem, unsigned int initial ) { - struct semaphore *sem; + sem->count = initial; +}
- if (!max || (initial > max)) - { - set_error( STATUS_INVALID_PARAMETER ); - return NULL; - } - if ((sem = create_named_object_dir( root, name, attr, &semaphore_ops ))) - { - if (get_error() != STATUS_OBJECT_NAME_EXISTS) - { - /* initialize it if it didn't already exist */ - sem->count = initial; - sem->max = max; - if (sd) default_set_sd( &sem->obj, sd, OWNER_SECURITY_INFORMATION| - GROUP_SECURITY_INFORMATION| - DACL_SECURITY_INFORMATION| - SACL_SECURITY_INFORMATION ); - } - } - return sem; +static inline unsigned int semaphore_get_value( struct semaphore *sem ) +{ + return sem->count; +} + +static inline void semaphore_down( struct semaphore *sem ) +{ + --sem->count; }
static int release_semaphore( struct semaphore *sem, unsigned int count, @@ -119,12 +147,187 @@ static int release_semaphore( struct semaphore *sem, unsigned int count, } return 1; } +#else /* !(ENABLE_POSIX_SYNC) */ + +static DWORD next_semaphore_id = 0; + +static void semaphore_new( struct semaphore *sem, unsigned int initial ) +{ + /* imperfect, but good enough for now */ + int tries; + for (tries = 0; tries < 0x10000; ++tries) + { + DWORD pid = (DWORD)getpid(); + DWORD reverse_pid = 0; + DWORD bit; + + for (bit = (1 << (sizeof(DWORD) * 8 - 1)); bit && pid; bit >>= 1, pid >>= 1) + { + if (pid & 1) + reverse_pid |= bit; + } + + sem->key = reverse_pid ^ next_semaphore_id++; + snprintf(sem->name, sizeof(sem->name) - 1, "/wine-sem-%08x", sem->key); + sem->p = sem_open(sem->name, O_CREAT | O_EXCL, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP, initial); + + if (sem->p != SEM_FAILED) + { + //fprintf(stderr, "semaphore_new: sem = %p, sem->p = %p, sem->name = %s\n", sem, sem->p, sem->name); + return; + } + + switch (errno) + { + case EACCES: + case EEXIST: + continue; + + case EINVAL: + set_error( STATUS_INVALID_PARAMETER ); + break; + case EMFILE: + case ENFILE: + set_error( STATUS_TOO_MANY_OPENED_FILES ); + break; + case ENOMEM: + set_error( STATUS_NO_MEMORY ); + break; + default: + break; + } + perror("sem_open"); + assert(0); + exit(1); + } + fprintf(stderr, "failed to create unique key for semaphore\n"); + assert(0); +} + +static unsigned int semaphore_get_value( struct semaphore *sem ) +{ + int ret; + + if (sem_getvalue(sem->p, &ret) == -1) + { + perror("sem_getvalue"); + exit(1); + + return STATUS_INVALID_HANDLE; + } + + return (unsigned int)ret; +} + +static void semaphore_destroy( struct object *obj ) +{ + struct semaphore *sem = (struct semaphore *)obj; + assert( obj->ops == &semaphore_ops ); + sem_destroy(sem->p); +} + +static int semaphore_trylock( struct object *obj, struct wait_queue_entry *entry ) +{ + struct semaphore *sem = (struct semaphore *)obj; + assert( obj->ops == &semaphore_ops ); + if (sem_trywait(sem->p) == -1) + { + switch(errno) { + case EAGAIN: + return 0; + + default: + perror("sem_trywait"); + exit(1); + } + } + return 1; +} + +static void semaphore_trylock_undo( struct object *obj, struct wait_queue_entry *entry ) +{ + struct semaphore *sem = (struct semaphore *)obj; + assert( obj->ops == &semaphore_ops ); + if (sem_post(sem->p) == -1) + { + perror("sem_post"); + exit(1); + } + +} + +static int release_semaphore( struct semaphore *sem, unsigned int count, + unsigned int *prev ) +{ + unsigned int cur_count; + + cur_count = semaphore_get_value(sem); + + if (prev) *prev = cur_count; + if (cur_count + count < cur_count || cur_count + count > sem->max) + { + set_error( STATUS_SEMAPHORE_LIMIT_EXCEEDED ); + return 0; + } + else + { + while(count--) + { + if (sem_post(sem->p) == -1) + { + /* FIXME: no atomic way to increase more than once */ + perror("sem_post"); + } + } + + /* there cannot be any thread to wake up if the count is != 0 */ + if (!cur_count) + wake_up( &sem->obj, count ); + } + return 1; +} + +#endif /* ENABLE_POSIX_SYNC */ + +static struct semaphore *create_semaphore( struct directory *root, const struct unicode_str *name, + unsigned int attr, unsigned int initial, unsigned int max, + const struct security_descriptor *sd ) +{ + struct semaphore *sem; + + if (!max || (initial > max)) + { + set_error( STATUS_INVALID_PARAMETER ); + return NULL; + } + if ((sem = create_named_object_dir( root, name, attr, &semaphore_ops ))) + { + if (get_error() != STATUS_OBJECT_NAME_EXISTS) + { + /* initialize it if it didn't already exist */ + semaphore_new(sem, initial); + sem->max = max; + if (sd) default_set_sd( &sem->obj, sd, OWNER_SECURITY_INFORMATION| + GROUP_SECURITY_INFORMATION| + DACL_SECURITY_INFORMATION| + SACL_SECURITY_INFORMATION ); + } +#if ENABLE_POSIX_SYNC + else + { + assert(sem->p); + assert(sem->name[0]); + } +#endif + } + return sem; +}
static void semaphore_dump( struct object *obj, int verbose ) { struct semaphore *sem = (struct semaphore *)obj; assert( obj->ops == &semaphore_ops ); - fprintf( stderr, "Semaphore count=%d max=%d ", sem->count, sem->max ); + fprintf( stderr, "Semaphore count=%d max=%d ", semaphore_get_value(sem), sem->max ); dump_object_name( &sem->obj ); fputc( '\n', stderr ); } @@ -140,15 +343,17 @@ static int semaphore_signaled( struct object *obj, struct wait_queue_entry *entr { struct semaphore *sem = (struct semaphore *)obj; assert( obj->ops == &semaphore_ops ); - return (sem->count > 0); + return (semaphore_get_value(sem) > 0); }
static void semaphore_satisfied( struct object *obj, struct wait_queue_entry *entry ) { +#ifndef ENABLE_POSIX_SYNC struct semaphore *sem = (struct semaphore *)obj; assert( obj->ops == &semaphore_ops ); - assert( sem->count ); - sem->count--; + assert( semaphore_get_value(sem) ); + semaphore_down(sem); +#endif }
static unsigned int semaphore_map_access( struct object *obj, unsigned int access ) @@ -199,6 +404,10 @@ DECL_HANDLER(create_semaphore) reply->handle = alloc_handle( current->process, sem, req->access, req->attributes ); else reply->handle = alloc_handle_no_access_check( current->process, sem, req->access, req->attributes ); +#ifdef ENABLE_POSIX_SYNC + reply->key = sem->key; +#endif + reply->server_ptr = (unsigned long)sem; release_object( sem ); }
@@ -219,6 +428,11 @@ DECL_HANDLER(open_semaphore) if ((sem = open_object_dir( root, &name, req->attributes, &semaphore_ops ))) { reply->handle = alloc_handle( current->process, &sem->obj, req->access, req->attributes ); +#ifdef ENABLE_POSIX_SYNC + reply->key = sem->key; +#endif + reply->max = sem->max; + reply->server_ptr = (unsigned long)sem; release_object( sem ); }
@@ -246,7 +460,7 @@ DECL_HANDLER(query_semaphore) if ((sem = (struct semaphore *)get_handle_obj( current->process, req->handle, SEMAPHORE_QUERY_STATE, &semaphore_ops ))) { - reply->current = sem->count; + reply->current = semaphore_get_value(sem); reply->max = sem->max; release_object( sem ); } diff --git a/server/thread.c b/server/thread.c index 6383000..4ed39d6 100644 --- a/server/thread.c +++ b/server/thread.c @@ -653,6 +653,12 @@ static int check_wait( struct thread *thread ) int i; struct thread_wait *wait = thread->wait; struct wait_queue_entry *entry; + const int posix_sync_enabled = +#ifdef ENABLE_POSIX_SYNC + 1; +#else + 0; +#endif
assert( wait );
@@ -664,12 +670,30 @@ static int check_wait( struct thread *thread )
if (wait->select == SELECT_WAIT_ALL) { - int not_ok = 0; + ULONGLONG ok = 0; + assert(MAXIMUM_WAIT_OBJECTS <= sizeof(ULONGLONG) * 8); /* Note: we must check them all anyway, as some objects may * want to do something when signaled, even if others are not */ for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) - not_ok |= !entry->obj->ops->signaled( entry->obj, entry ); - if (not_ok) goto other_checks; + { + int signaled = posix_sync_enabled && entry->obj->ops->trylock + ? entry->obj->ops->trylock( entry->obj, entry ) + : entry->obj->ops->signaled( entry->obj, entry ); + ok |= (ULONGLONG)(!!signaled) << i; + } + if (!ok) + { + if (posix_sync_enabled) + { + /* reverse iterate and undo any sucessful trylocks */ + for (--i, --entry; i >= 0; --i, --entry) + { + if ((ok & (1LL << i)) && entry->obj->ops->trylock) + entry->obj->ops->trylock_undo( entry->obj, entry ); + } + } + goto other_checks; + } /* Wait satisfied: tell it to all objects */ for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) entry->obj->ops->satisfied( entry->obj, entry ); @@ -679,7 +703,11 @@ static int check_wait( struct thread *thread ) { for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) { - if (!entry->obj->ops->signaled( entry->obj, entry )) continue; + int signaled = posix_sync_enabled && entry->obj->ops->trylock + ? entry->obj->ops->trylock( entry->obj, entry ) + : entry->obj->ops->signaled( entry->obj, entry ); + + if (!signaled) continue; /* Wait satisfied: tell it to the object */ entry->obj->ops->satisfied( entry->obj, entry ); if (wait->abandoned) i += STATUS_ABANDONED_WAIT_0;
Adds a facility for ntdll to track handles locally and attach information to them, allowing for some operations to be performed directly by the client process.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- dlls/ntdll/loader.c | 1 + dlls/ntdll/ntdll_misc.h | 61 +++++++++++ dlls/ntdll/om.c | 264 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 326 insertions(+)
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 8a43310..0c779e2 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -3258,6 +3258,7 @@ void __wine_process_init(void) umask( FILE_umask );
load_global_options(); + ntdll_object_db_init();
/* setup the load callback and create ntdll modref */ wine_dll_set_callback( load_builtin_callback ); diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h index cbd19db..08e48d7 100644 --- a/dlls/ntdll/ntdll_misc.h +++ b/dlls/ntdll/ntdll_misc.h @@ -28,6 +28,8 @@ #include "winnt.h" #include "winternl.h" #include "wine/server.h" +#include "wine/rbtree.h" +#include "wine/list.h"
#define MAX_NT_PATH_LENGTH 277
@@ -268,4 +270,63 @@ extern HANDLE keyed_event DECLSPEC_HIDDEN;
NTSTATUS WINAPI RtlHashUnicodeString(PCUNICODE_STRING,BOOLEAN,ULONG,ULONG*);
+enum ntdll_obj_type { + NTDLL_OBJ_TYPE_ASYNC, /* async I/O */ + NTDLL_OBJ_TYPE_EVENT, /* event */ + NTDLL_OBJ_TYPE_COMPLETION, /* IO completion ports */ + NTDLL_OBJ_TYPE_FILE, /* file */ + NTDLL_OBJ_TYPE_MAILSLOT, /* +2 chain mail slot */ + NTDLL_OBJ_TYPE_MUTEX, /* mutex */ + NTDLL_OBJ_TYPE_MSG_QUEUE, /* message queue */ + NTDLL_OBJ_TYPE_PROCESS, /* process */ + NTDLL_OBJ_TYPE_SEMAPHORE, /* semaphore */ + NTDLL_OBJ_TYPE_THREAD, /* thread */ + NTDLL_OBJ_TYPE_WAITABLE_TIMER, /* waitable timer */ + + NTDLL_OBJ_TYPE_MAX +}; + +struct ntdll_object; + +struct ntdll_object_ops { + /* wait on the object (TODO) */ + NTSTATUS (*wait) (struct ntdll_object *obj, const LARGE_INTEGER *timeout); + /* attempt to obtain lock on object, return STATUS_SUCCESS or STATUS_WAS_LOCKED upon failure. */ + NTSTATUS (*trywait) (struct ntdll_object *obj); + /* called to release a lock previous obtained by trywait() */ + NTSTATUS (*trywait_undo)(struct ntdll_object *obj); + /* close the client-side object */ + void (*close) (struct ntdll_object *obj); + /* Dump a description of the object to the supplied buffer. The implementation should + * generally call ntdll_object_dump_base() to describe it's struct ntdll_object member.*/ + void (*dump) (const struct ntdll_object *obj, char **start, const char *const end); +}; + +/* process-local data for ntdll objects + * + * currently, fields ops - server_ptr should sit on one cache line in all cases, + * list_entry used infrequently */ +struct ntdll_object { + struct ntdll_object_ops *ops; /* ops */ + int refcount; /* reference count */ + HANDLE h; /* handle to the object (may be more than one handle tot he same server-side obj) */ + enum ntdll_obj_type type_id; /* redundant? maybe but keep for debugging for now */ + struct wine_rb_entry tree_entry; /* red-black tree node */ + ULONG_PTR server_ptr; /* for debugging only */ + struct list list_entry; +}; + +extern NTSTATUS ntdll_object_db_init(void); +extern struct ntdll_object *ntdll_object_new(const HANDLE h, size_t size, enum ntdll_obj_type type_id, struct ntdll_object_ops *ops); +extern struct ntdll_object *ntdll_object_grab(struct ntdll_object *obj); +extern void ntdll_object_release(struct ntdll_object *obj); +extern void ntdll_object_dump_base(const struct ntdll_object *obj, char **start, const char *const end); +extern const char *ntdll_object_dump(const struct ntdll_object *obj); +extern void *ntdll_object_get_debug_buffer(size_t *size); +extern int ntdll_handle_add(struct ntdll_object *obj); +extern void ntdll_handle_remove(const HANDLE h); +extern struct ntdll_object *ntdll_handle_find(const HANDLE h); + +extern void ntdll_server_notify_lock_release(void); + #endif diff --git a/dlls/ntdll/om.c b/dlls/ntdll/om.c index 3fadba7..6233633 100644 --- a/dlls/ntdll/om.c +++ b/dlls/ntdll/om.c @@ -20,6 +20,7 @@ */
#include "config.h" +#include "wine/port.h"
#include <stdarg.h> #include <stdlib.h> @@ -30,6 +31,9 @@ #ifdef HAVE_UNISTD_H # include <unistd.h> #endif +#include <assert.h> +#include <limits.h> +#include <stdio.h>
#include "ntstatus.h" #define WIN32_NO_STATUS @@ -40,6 +44,263 @@ #include "wine/server.h"
WINE_DEFAULT_DEBUG_CHANNEL(ntdll); +WINE_DECLARE_DEBUG_CHANNEL(ntdll_obj); + + +/* + * Process-local object information management + * + * Theory of Operation + * =================== + * Information about some ntdll objects are cached locally and used to facilitate + * IPC operations natively, keeping the server informed about anything important + * via asynchronous messages, preventing the need for the big context switch. This + * works for releasing locks and attempting to wait on one or more supported objects. + * All other WaitFor* calls are routed to the server. + * + * Lifecycle of a struct ntdll_object object + * + * refcount Operation + * (no object) Execute a server call that creates an object & returns a handle. + * 1 Call ntdll_object_new() passing handle to allocate a new struct ntdll_object. + * * allocates & zeros struct ntdll_object + * * adds to objects.list (double-linked list) + * 1 Initialize derived-type data members + * 2 Call ntdll_handle_add() to index handle in handles.tree (red-black tree) + * 1 Call ntdll_object_release() when done with the object. + * .... + * (no object) We receive an API call that we might be able to manage client-side + * 2 Call ntdll_handle_find() on the handle and get the object + * 2 Perform whatever we need to do locally + * 1 Call ntdll_object_release() when done with the object. + * ... + * (no object) We receive a call to NtClose (CloseHandle) + * 2 Call ntdll_handle_find() on the handle and get the object + * 1 Call ntdll_handle_remove() to remove it from handles.tree. + * 0 Call ntdll_object_release() + * * object is removed from list + * * refcount reaches zero and destructor ops->close() is called + * * memory is freed + * + * Because we're doing this in the multi-threaded environment of the client process instead of + * the safety of the single-threaded server, things won't always go as above. It is possible + * for a call to NtClose to be received while another thread is still using the object, so + * that the object returned by Call ntdll_handle_find() may have a refcount of 3 or more. + * However, the object should be immediately delisted, so we can have many live objects + * with the same handle, but only one of those handles (the one in the tree) is the living + * one. + * + * TODO: Make sure that UB is acceptable for when a program tries to use a stale handle, or + * should we zero the handle when it's removed from the tree (NtClose returns). + */ + + +/* FIXME: portability */ +static __thread char *tls_debug_buffer = NULL; +#define NTDLL_DEBUG_BUFFER_SIZE 0x400 + +static const char * const obj_type_desc[NTDLL_OBJ_TYPE_MAX] = { + "async I/O", /* NTDLL_OBJ_TYPE_ASYNC */ + "event", /* NTDLL_OBJ_TYPE_EVENT */ + "I/O completion ports", /* NTDLL_OBJ_TYPE_COMPLETION */ + "file", /* NTDLL_OBJ_TYPE_FILE */ + "mail slot", /* NTDLL_OBJ_TYPE_MAILSLOT */ + "mutex", /* NTDLL_OBJ_TYPE_MUTEX */ + "message queue", /* NTDLL_OBJ_TYPE_MSG_QUEUE */ + "process", /* NTDLL_OBJ_TYPE_PROCESS */ + "semaphore", /* NTDLL_OBJ_TYPE_SEMAPHORE */ + "thread", /* NTDLL_OBJ_TYPE_THREAD */ + "waitable timer", /* NTDLL_OBJ_TYPE_WAITABLE_TIMER */ +}; + +/* List containing all process-locally known objects */ +static struct { + struct list list; /* list of all struct ntdll_object ojbects */ + pthread_mutex_t mutex; /* lock for modifying list */ +} objects = { + {NULL, NULL}, + PTHREAD_MUTEX_INITIALIZER, /* non-recursive fast lock */ +}; + +/* Allocates and initialises a struct ntdll_object object and adds it to + * process-global list. Initial refcount is 1, so ntdll_object_release() + * must be called when finished initing derived-type members. + * + * Locking: + * Locks objects.mutex */ +struct ntdll_object *ntdll_object_new(const HANDLE h, size_t size, enum ntdll_obj_type type_id, struct ntdll_object_ops *ops) +{ + struct ntdll_object *obj; + + assert(size >= sizeof(*obj)); + assert(type_id < NTDLL_OBJ_TYPE_MAX); + + TRACE_(ntdll_obj)("%p, %zu, %u (%s), %p\n", h, size, type_id, obj_type_desc[type_id], ops); + + obj = RtlAllocateHeap(GetProcessHeap(), 0, size); + if (obj) + { + memset(obj, 0, size); + obj->h = h; + obj->refcount = 1; + obj->type_id = type_id; + obj->ops = ops; + obj->server_ptr = 0; + pthread_mutex_lock(&objects.mutex); + list_add_tail(&objects.list, &obj->list_entry); + pthread_mutex_unlock(&objects.mutex); + } else + ERR("Failed to alloc %zu bytes\n", sizeof(*obj)); + + return obj; +} + +struct ntdll_object *ntdll_object_grab(struct ntdll_object *obj) +{ + int refcount = interlocked_xchg_add(&obj->refcount, 1); + assert(refcount < INT_MAX && refcount > 0); + return obj; +} + +void ntdll_object_release(struct ntdll_object *obj) +{ + int refcount = interlocked_xchg_add(&obj->refcount, -1); + assert(refcount > 0); + + if (!refcount) + { + /* FIXME: check for any more race conditions */ + TRACE_(ntdll_obj)("Removing & freeing object %s\n", ntdll_object_dump(obj)); + pthread_mutex_lock(&objects.mutex); + list_remove(&obj->list_entry); + pthread_mutex_unlock(&objects.mutex); + if (obj->ops->close) + obj->ops->close(obj); + RtlFreeHeap(GetProcessHeap(), 0, obj); + } +} + +/* called by derived class to dump struct ntdll_object to a string buffer */ +void ntdll_object_dump_base(const struct ntdll_object *obj, char **start, const char *const end) +{ + int count; + + assert(start && *start && end); + assert(end > *start); + + if (!obj) + count = snprintf(*start, end - *start, "(NULL)"); + else + { + assert(obj->type_id < NTDLL_OBJ_TYPE_MAX); + + count = snprintf(*start, end - *start, + "%p {" + "ops = %p, " + "refcount = %u, " + "h = %p, " + "type_id = %u (%s), " + "tree_entry = {left = %p, right = %p, flags = 0x%x}, " + "server_ptr = %lx, " + "list_entry = {next = %p, prev = %p}" + "}", + obj, + obj->ops, + obj->refcount, + obj->h, + obj->type_id, obj_type_desc[obj->type_id], + obj->tree_entry.left, obj->tree_entry.right, obj->tree_entry.flags, + obj->server_ptr, + obj->list_entry.next, obj->list_entry.prev); + } + + if (count < 0) + { + perror("snprintf"); + return; + } + + *start += count; +} + +const char *ntdll_object_dump(const struct ntdll_object *obj) +{ + char *start; + const char *ret; + size_t size; + + if (!(start = ntdll_object_get_debug_buffer(&size))) + return NULL; + + ret = start; + if (!obj) + snprintf(start, size, "(NULL)"); + else if (obj->ops->dump) + obj->ops->dump(obj, &start, start + size); + else + ntdll_object_dump_base(obj, &start, start + size); + + return ret; +} + +/* return a thread-local buffer for dumping object descriptions to */ +void *ntdll_object_get_debug_buffer(size_t *size) +{ + if (!tls_debug_buffer) + tls_debug_buffer = RtlAllocateHeap(GetProcessHeap(), 0, NTDLL_DEBUG_BUFFER_SIZE); + + if (size) + *size = tls_debug_buffer ? NTDLL_DEBUG_BUFFER_SIZE : 0; + return tls_debug_buffer; +} + + +/* red-black tree functions for handle table */ + +static inline void *ntdll_object_rb_alloc(size_t size) +{ + return RtlAllocateHeap(GetProcessHeap(), 0, size); +} + +static inline void *ntdll_object_rb_realloc(void *ptr, size_t size) +{ + return RtlReAllocateHeap(GetProcessHeap(), 0, ptr, size); +} + +static inline void ntdll_object_rb_free(void *ptr) +{ + RtlFreeHeap(GetProcessHeap(), 0, ptr); +} + +static inline int ntdll_object_handle_compare(const void *key, const struct wine_rb_entry *entry) +{ + const HANDLE *_a = key; + const HANDLE *_b = &WINE_RB_ENTRY_VALUE(entry, const struct ntdll_object, tree_entry)->h; + +#ifdef __GNUC__ + /* when inlined this is more efficient than "_a - _b" and GCC 4.7+ should inline it */ + return *_a > *_b ? 1 : (*_a < *_b ? -1 : 0); +#else + return *_a - *_b; +#endif +} + +static const struct wine_rb_functions obj_handles_rb_ops = +{ + ntdll_object_rb_alloc, + ntdll_object_rb_realloc, + ntdll_object_rb_free, + ntdll_object_handle_compare, +}; + +/* Tree mapping all process-locally known kernel handles to a struct ntdll_object */ +static struct { + struct wine_rb_tree tree; + pthread_mutex_t mutex; +} handles = { + { &obj_handles_rb_ops, NULL, {NULL, 0, 0}}, /* static initializer to aid -findirect-inline */ + PTHREAD_MUTEX_INITIALIZER /* non-recursive fast lock */ +};
/* @@ -390,6 +651,9 @@ NTSTATUS close_handle( HANDLE handle ) } SERVER_END_REQ; if (fd != -1) close( fd ); + + ntdll_handle_remove( handle ); + return ret; }
Adds a facility for ntdll to track handles locally and attach information to them, allowing for some operations to be performed directly by the client process.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- dlls/ntdll/om.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 186 insertions(+)
diff --git a/dlls/ntdll/om.c b/dlls/ntdll/om.c index 6233633..dfb0f76 100644 --- a/dlls/ntdll/om.c +++ b/dlls/ntdll/om.c @@ -302,6 +302,192 @@ static struct { PTHREAD_MUTEX_INITIALIZER /* non-recursive fast lock */ };
+/* Adds the object to the handle database, increasing its reference count to account for + * it being referenced by the handle tree its self (don't ntdll_object_release it to + * resolve this increment). + * + * Locking: + * Locks handles.mutex */ +__attribute__((noinline, flatten)) +int ntdll_handle_add(struct ntdll_object *obj) +{ + int ret = 0; + + TRACE_(ntdll_obj)("%p\n", obj); + +try_again: + pthread_mutex_lock(&handles.mutex); + ret = wine_rb_put(&handles.tree, &obj->h, &obj->tree_entry); + + /* increase the ref count */ + if (!ret) + ntdll_object_grab(obj); + pthread_mutex_unlock(&handles.mutex); + + if (ret) + { + struct ntdll_object *existing = ntdll_handle_find(obj->h); + + /* bug if we get here */ + ERR("Failed to insert handle %p.\n", obj->h); + if (existing) + ERR("Existing object: %s\n", ntdll_object_dump(existing)); + ERR("New object: %s\n", ntdll_object_dump(obj)); + assert(0); + + /* should probably just exit(1) here */ + ntdll_handle_remove(obj->h); + goto try_again; + } + + TRACE("Added obj %s\n", ntdll_object_dump(obj)); + + return ret; +} + +/* Remove an object from the handles tree. If no object with the specified handle is found + * then nothing is done. If an object is found but it's refcount is 0 then a failed assertion + * will result. + * + * Locking: + * Locks handles.mutex */ +__attribute__((noinline)) +void ntdll_handle_remove(const HANDLE h) +{ + struct ntdll_object *obj = ntdll_handle_find(h); + + if (!obj) + return; + + TRACE_(ntdll_obj)("h = %p) obj = %s\n", h, ntdll_object_dump(obj)); + assert(obj->refcount >= 1); + + pthread_mutex_lock(&handles.mutex); + /* FIXME: a more efficient wine_rb_remove_by_entry would be nice here */ + wine_rb_remove(&handles.tree, &h); + pthread_mutex_unlock(&handles.mutex); + + ntdll_object_release(obj); +} + + + +/* Searches for an object in the process-local database with the specified handle. + * If found, the object's refcount is incremented and a pointer to the object is + * returned -- it will then be the responsibility of the caller to release the + * object when done. + * + * Locking: + * Locks handles.mutex */ +struct ntdll_object *ntdll_handle_find(const HANDLE h) +{ + struct wine_rb_entry *entry; + struct ntdll_object *ret = NULL; + + pthread_mutex_lock(&handles.mutex); + entry = wine_rb_get(&handles.tree, &h); + + if (entry) + { + ret = WINE_RB_ENTRY_VALUE(entry, struct ntdll_object, tree_entry); + ntdll_object_grab(ret); + } + pthread_mutex_unlock(&handles.mutex); + + //TRACE_(ntdll_obj)("(%p) result: %p\n", h, ret); + + return ret; +} + + +/* callback for ntdll_objects_cleanup() + * + * Locking: + * Locks objects.mutex + * Called when handles.mutex already locked + */ + +static void ntdll_objects_cleanup_cb(struct wine_rb_entry *entry, void *context) +{ + struct ntdll_object *obj = WINE_RB_ENTRY_VALUE(entry, struct ntdll_object, tree_entry); + size_t *leaked_handles = (size_t *)context; + + ++(*leaked_handles); + + ERR_(ntdll_obj)("Leaked object handle: %s\n", ntdll_object_dump(obj)); + + /* handles.mutex already locked */ + wine_rb_remove(&handles.tree, &obj->h); + + ntdll_object_release(obj); +} + +/* atexit() cleanup + * + * Locking: + * Locks handles.mutex --> then objects.mutex + */ +static void ntdll_objects_cleanup(void) +{ + size_t leaked_handles = 0; + size_t leaked_objects = 0; + + TRACE_(ntdll_obj)("\n"); + pthread_mutex_lock(&handles.mutex); + wine_rb_for_each_entry(&handles.tree, ntdll_objects_cleanup_cb, &leaked_handles); + pthread_mutex_unlock(&handles.mutex); + + pthread_mutex_lock(&objects.mutex); + leaked_objects = list_count(&objects.list); + if (leaked_objects) + { + struct ntdll_object *obj; + //struct list *i; + LIST_FOR_EACH_ENTRY( obj, &objects.list, struct ntdll_object , list_entry ) + { + ERR_(ntdll_obj)("Leaked object: %s\n", ntdll_object_dump(obj)); + } + } + pthread_mutex_unlock(&objects.mutex); + + if (leaked_handles || leaked_objects) + ERR_(ntdll_obj)("*** %zu leaked handles found, %zu leaked objects remain.\n", + leaked_handles, leaked_objects); + +} + +/* initialize objects list and handles tree */ +NTSTATUS ntdll_object_db_init(void) +{ + NTSTATUS ret = 0; + + TRACE_(ntdll_obj)("\n"); + + /* init objects list if not already inited */ + pthread_mutex_lock(&objects.mutex); + if (!objects.list.next) + list_init(&objects.list); + pthread_mutex_unlock(&objects.mutex); + + /* init red-black handle-to-object tree if not already inited */ + pthread_mutex_lock(&handles.mutex); + if (!handles.tree.stack.entries) + { + /* passing handles.tree.functions instead of obj_handles_rb_ops to aid -findirect-inline + * (it might not matter) */ + if (wine_rb_init(&handles.tree, handles.tree.functions) == -1) + { + ERR("Failed to initialize ntdll object handle rbtree.\n"); + ret = ERROR_OUTOFMEMORY; + } + else + atexit(ntdll_objects_cleanup); + } + pthread_mutex_unlock(&handles.mutex); + + return ret; +} +
/* * Generic object functions
When a semaphore is opened or created, the client process now also opens that semaphore and can perform operations on it
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- dlls/ntdll/sync.c | 232 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 232 insertions(+)
diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c index 6892732..84a3ab6 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -48,6 +48,19 @@ #include <stdlib.h> #include <time.h>
+#if ENABLE_POSIX_SYNC +# ifdef HAVE_SEMAPHORE_H +# include <semaphore.h> +# endif +# include <fcntl.h> +# include <sys/stat.h> +# include <semaphore.h> +# include <errno.h> +# ifdef HAVE_SYS_TYPES_H +# include <sys/types.h> +# endif +#endif + #include "ntstatus.h" #define WIN32_NO_STATUS #define NONAMELESSUNION @@ -58,9 +71,226 @@ #include "ntdll_misc.h"
WINE_DEFAULT_DEBUG_CHANNEL(ntdll); +WINE_DECLARE_DEBUG_CHANNEL(ntdllsync);
HANDLE keyed_event = NULL;
+static NTSTATUS semaphore_add(const HANDLE h, LONG MaximumCount, unsigned int key, unsigned long debug_server_ptr); + +#if !(ENABLE_POSIX_SYNC) +static NTSTATUS semaphore_add(const HANDLE h, LONG MaximumCount, unsigned int key, unsigned long debug_server_ptr) +{ + return STATUS_SUCCESS; +} + +static inline NTSTATUS semaphore_up(const HANDLE h, ULONG count, PULONG previous) +{ + return STATUS_NOT_IMPLEMENTED; +} +#else /* ENABLE_POSIX_SYNC */ + +struct ntdll_semaphore { + struct ntdll_object obj; + sem_t *p; + LONG max; + unsigned int key; + unsigned long server_ptr; +}; + +static void semaphore_dump(const struct ntdll_object *obj, char **start, const char *const end); + +#if 0 +static int sem_ +sem_getvalue + if (sem_getvalue(sem->p, &ret) == -1) + { + perror("sem_getvalue"); + exit(1); + + return STATUS_INVALID_HANDLE; + } + + return (unsigned int)ret; +} +#endif + +static NTSTATUS semaphore_up(struct ntdll_semaphore *sem, ULONG count, PULONG previous) +{ + int current_value; + + TRACE_(ntdllsync)("(sem = %s, count = %u, previous = %p)\n", + ntdll_object_dump(&sem->obj), count, previous); + + if (sem_getvalue(sem->p, ¤t_value) == -1) + { + perror("sem_getvalue"); + assert(0); + ERR_(ntdllsync)("sem_getvalue failed.\n"); + + return STATUS_INVALID_HANDLE; /* ?? */ + } + + if (previous) + *previous = current_value; + + if (current_value + count > sem->max) + return STATUS_SEMAPHORE_LIMIT_EXCEEDED; + + while (count--) + { + /* BUG by multiple threads releasing at the same time, it is possible to exceed sem->max here */ + if (sem_post(sem->p) == -1) + { + perror("sem_post"); + /* ?? */ + } + } + TRACE_(ntdllsync)("success\n"); + + ntdll_server_notify_lock_release(); + return STATUS_SUCCESS; +} + +/* client-side waiting not yet implemented */ +static NTSTATUS semaphore_wait(struct ntdll_object *obj, const LARGE_INTEGER *timeout) +{ + TRACE_(ntdllsync)("(obj = %s, timeout = %p)\n", ntdll_object_dump(obj), timeout); + + return STATUS_NOT_IMPLEMENTED; +} + +static NTSTATUS semaphore_trywait(struct ntdll_object *obj) +{ + struct ntdll_semaphore *sem = (void *)obj; + + TRACE_(ntdllsync)("(obj = %s)\n", ntdll_object_dump(obj)); + + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + assert(sem->p); + + if (sem_trywait(sem->p)) + { + switch (errno) { + case EAGAIN: + return STATUS_WAS_LOCKED; + case EDEADLK: + return STATUS_POSSIBLE_DEADLOCK; + default: + perror("sem_trywait"); + exit(1); + } + } + return STATUS_SUCCESS; +} + +static NTSTATUS semaphore_trywait_undo(struct ntdll_object *obj) +{ + struct ntdll_semaphore *sem = (void *)obj; + + TRACE_(ntdllsync)("(obj = %s)\n", ntdll_object_dump(obj)); + + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + assert(sem->p); + + if (sem_post(sem->p) == -1) + { + perror("sem_post"); + exit(1); + } + return STATUS_SUCCESS; +} + +static void semaphore_close(struct ntdll_object *obj) +{ + struct ntdll_semaphore *sem = (void *)obj; + + TRACE_(ntdllsync)("(obj = %s)\n", ntdll_object_dump(obj)); + + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + assert(sem->p); + + if (sem->p) + sem_close(sem->p); +} + +static void semaphore_dump(const struct ntdll_object *obj, char **start, const char *const end) +{ + struct ntdll_semaphore *sem = (void *)obj; + int count; + int sem_value; + + assert(sem); + assert(sem->p); + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + + count = snprintf(*start, end - *start, "%p {obj = ", obj); + if (count < 0) + { + perror("snprintf"); + return; + } + *start += count; + + ntdll_object_dump_base(obj, start, end); + + if (sem_getvalue(sem->p, &sem_value) == -1) + sem_value = -1; + + count = snprintf(*start, end - *start, + ", p = %p {value = %d}, max = %d, key = %08x, server_ptr = %p", + sem->p, sem_value, sem->max, sem->key, (void*)sem->server_ptr); + if (count < 0) + perror("snprintf"); +} + +static struct ntdll_object_ops semaphore_ops = { + semaphore_wait, /* wait */ + semaphore_trywait, /* trywait */ + semaphore_trywait_undo, /* trywait_undo */ + semaphore_close, /* close */ + semaphore_dump /* dump */ +}; + +static NTSTATUS semaphore_add(const HANDLE h, LONG MaximumCount, unsigned int key, unsigned long server_ptr) +{ + struct ntdll_semaphore *sem; + sem_t *sem_obj; + char name[32]; + + snprintf(name, sizeof(name) - 1, "/wine-sem-%08x", key); + TRACE_(ntdllsync)("(h = %p, MaximumCount = %d, key = %08x, name = "%s")\n", h, MaximumCount, key, name); + sem_obj = sem_open(name, 0); + + if (sem_obj == SEM_FAILED) + { + perror("sem_open"); + assert(0); + exit(1); + } + + sem = (void *)ntdll_object_new(h, sizeof(struct ntdll_semaphore), NTDLL_OBJ_TYPE_SEMAPHORE, &semaphore_ops); + + if (!sem) + { + ERR_(ntdllsync)("ntdll_object_new failed\n"); + sem_close(sem_obj); + return STATUS_NO_MEMORY; + } + sem->max = MaximumCount; + sem->p = sem_obj; + sem->key = key; + sem->server_ptr = server_ptr; + TRACE_(ntdllsync)("%s\n", ntdll_object_dump(&sem->obj)); + + ntdll_handle_add(&sem->obj); + ntdll_object_release(&sem->obj); + + return STATUS_SUCCESS; +} + +#endif /* ENABLE_POSIX_SYNC */ + + static inline int interlocked_dec_if_nonzero( int *dest ) { int val, tmp; @@ -184,6 +414,7 @@ NTSTATUS WINAPI NtCreateSemaphore( OUT PHANDLE SemaphoreHandle, if (len) wine_server_add_data( req, attr->ObjectName->Buffer, len ); ret = wine_server_call( req ); *SemaphoreHandle = wine_server_ptr_handle( reply->handle ); + semaphore_add(*SemaphoreHandle, MaximumCount, reply->key, (unsigned long)reply->server_ptr); } SERVER_END_REQ;
@@ -212,6 +443,7 @@ NTSTATUS WINAPI NtOpenSemaphore( OUT PHANDLE SemaphoreHandle, if (len) wine_server_add_data( req, attr->ObjectName->Buffer, len ); ret = wine_server_call( req ); *SemaphoreHandle = wine_server_ptr_handle( reply->handle ); + semaphore_add(*SemaphoreHandle, reply->max, reply->key, (unsigned long)reply->server_ptr); } SERVER_END_REQ; return ret;
When enabled, ReleaseSemaphore will be done by the client process without a server call. However, notification to the server is required and a rather ugly but temporary SIGUSR1 mechanism is implemented (will be changed soon).
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- dlls/ntdll/server.c | 14 ++++++++++++++ dlls/ntdll/sync.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+)
diff --git a/dlls/ntdll/server.c b/dlls/ntdll/server.c index 95111ad..763a90d 100644 --- a/dlls/ntdll/server.c +++ b/dlls/ntdll/server.c @@ -682,6 +682,20 @@ unsigned int server_queue_process_apc( HANDLE process, const apc_call_t *call, a } }
+/* notify server that we've released a lock as it won't actually wait (block) + * on any locks and may have a thread suspended waiting for this lock to be + * released */ +void ntdll_server_notify_lock_release(void) +{ + TRACE_(ntdllsync)("server_pid = %u\n", server_pid); + if (server_pid && server_pid != -1) + { + if (kill(server_pid, SIGUSR1)) + { + perror("kill"); + } + } +}
/*********************************************************************** * wine_server_send_fd (NTDLL.@) diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c index 84a3ab6..a26e40c 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -487,6 +487,37 @@ NTSTATUS WINAPI NtQuerySemaphore( HANDLE handle, SEMAPHORE_INFORMATION_CLASS cla NTSTATUS WINAPI NtReleaseSemaphore( HANDLE handle, ULONG count, PULONG previous ) { NTSTATUS ret; + + struct ntdll_semaphore *sem = (void *)ntdll_handle_find(handle); + + /* if we know this handle, then manage it client-side, otherwise, it may be a stale handle + * that another thread already closed */ + if (sem) + { + TRACE_(ntdllsync)("handle = %p, count = %u, previous = %p) sem = %s\n", + handle, count, previous, ntdll_object_dump(&sem->obj)); + + assert(sem->p); + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + + ret = semaphore_up(sem, count, previous); + ntdll_object_release(&sem->obj); + + switch (ret) + { + case STATUS_SUCCESS: + return STATUS_SUCCESS; + + case STATUS_NOT_IMPLEMENTED: + /* fall through to server call */ + break; + + default: + ERR("semaphore_up return %08x\n", ret); + return ret; + } + } + SERVER_START_REQ( release_semaphore ) { req->handle = wine_server_obj_handle( handle );
Sever will now handle SIGUSR1 as notifcation that a client process has released a lock and will walk all threads to see if any are ready to run. This will be replaced by a proper asynchronous message where the client will tell the server exactly which object was released.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- server/signal.c | 12 ++++++++++++ server/thread.c | 20 ++++++++++++++++++++ server/thread.h | 1 + 3 files changed, 33 insertions(+)
diff --git a/server/signal.c b/server/signal.c index 5e4fe33..f241604 100644 --- a/server/signal.c +++ b/server/signal.c @@ -98,6 +98,7 @@ static struct handler *handler_sigterm; static struct handler *handler_sigint; static struct handler *handler_sigchld; static struct handler *handler_sigio; +static struct handler *handler_sigusr1;
static int watchdog;
@@ -239,6 +240,11 @@ static void do_sigio( int signum, siginfo_t *si, void *x ) } #endif
+static void do_sigusr1( int signum ) +{ + do_signal( handler_sigusr1 ); +} + void start_watchdog(void) { alarm( 3 ); @@ -277,6 +283,7 @@ void init_signals(void) if (!(handler_sigint = create_handler( sigint_callback ))) goto error; if (!(handler_sigchld = create_handler( sigchld_callback ))) goto error; if (!(handler_sigio = create_handler( sigio_callback ))) goto error; + if (!(handler_sigusr1 = create_handler( sigusr1_callback ))) goto error;
sigemptyset( &blocked_sigset ); sigaddset( &blocked_sigset, SIGCHLD ); @@ -289,6 +296,7 @@ void init_signals(void) #ifdef SIG_PTHREAD_CANCEL sigaddset( &blocked_sigset, SIG_PTHREAD_CANCEL ); #endif + sigaddset( &blocked_sigset, SIGUSR1 );
action.sa_mask = blocked_sigset; action.sa_flags = 0; @@ -318,6 +326,10 @@ void init_signals(void) action.sa_flags = SA_SIGINFO; sigaction( SIGIO, &action, NULL ); #endif + action.sa_handler = do_sigusr1; + action.sa_flags = 0; + sigaction( SIGUSR1, &action, NULL ); + return;
error: diff --git a/server/thread.c b/server/thread.c index 4ed39d6..b4edfd9 100644 --- a/server/thread.c +++ b/server/thread.c @@ -721,6 +721,26 @@ static int check_wait( struct thread *thread ) return -1; }
+/* a less-than-elegant solution to waking up threads after a thread releases a native + * lock and sends us SIGUSR1 */ +static int quick_n_dirty_check_wait_threads(struct process *p, void *smart_dummy) +{ + struct thread *t; + + LIST_FOR_EACH_ENTRY( t, &p->thread_list, struct thread, proc_entry ) + { + if (t->wait) + wake_thread ( t ); + } + + return 0; +} + +void sigusr1_callback(void) +{ + enum_processes(quick_n_dirty_check_wait_threads, NULL); +} + /* send the wakeup signal to a thread */ static int send_thread_wakeup( struct thread *thread, client_ptr_t cookie, int signaled ) { diff --git a/server/thread.h b/server/thread.h index 2821991..40f3e2f 100644 --- a/server/thread.h +++ b/server/thread.h @@ -127,6 +127,7 @@ extern struct thread_snapshot *thread_snap( int *count ); extern struct token *thread_get_impersonation_token( struct thread *thread ); extern int set_thread_affinity( struct thread *thread, affinity_t affinity ); extern int is_cpu_supported( enum cpu_type cpu ); +extern void sigusr1_callback(void);
/* ptrace functions */
server_select can now trywait on native semaphores in a select_op when wait conditions do not require the thread to block. Otherwise, a standard server call is made.
Signed-off-by: Daniel Santos daniel.santos@pobox.com --- dlls/ntdll/server.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+)
diff --git a/dlls/ntdll/server.c b/dlls/ntdll/server.c index 763a90d..b186791 100644 --- a/dlls/ntdll/server.c +++ b/dlls/ntdll/server.c @@ -83,6 +83,7 @@ #include "ntdll_misc.h"
WINE_DEFAULT_DEBUG_CHANNEL(server); +WINE_DECLARE_DEBUG_CHANNEL(ntdllsync);
/* Some versions of glibc don't define this */ #ifndef SCM_RIGHTS @@ -594,6 +595,138 @@ unsigned int server_select( const select_op_t *select_op, data_size_t size, UINT
memset( &result, 0, sizeof(result) );
+#if ENABLE_POSIX_SYNC + TRACE_(ntdllsync)("select_op = %p, size = %u, flags = 0x%x, timeout = %p (%zd)\n", + select_op, size, flags, timeout, (timeout ? timeout->QuadPart : 0)); + + if (select_op && (select_op->op == SELECT_WAIT || select_op->op == SELECT_WAIT_ALL)) + { + struct ntdll_object *objs[MAXIMUM_WAIT_OBJECTS]; + size_t nb_locally_lockable_objs = 0; + size_t nb_handles = (size - offsetof( select_op_t, wait.handles )) + / sizeof(select_op->wait.handles[0]); + ssize_t i; + NTSTATUS result; + NTSTATUS ret = STATUS_UNSUCCESSFUL; + ssize_t wait_object = 0; + + /* count the number of locally lockable objects & cache their pointers */ + for (i = 0; i < nb_handles; ++i) + { + objs[i] = ntdll_handle_find(wine_server_ptr_handle(select_op->wait.handles[i])); + if (objs[i] && objs[i]->ops->trywait) + ++nb_locally_lockable_objs; + } + + TRACE_(ntdllsync)("%zd/%zd objects are locally selectable, need %s.\n", + nb_locally_lockable_objs, nb_handles, + (select_op->op == SELECT_WAIT ? "any" : "all")); + + /* bWaitAll = FALSE */ + if (select_op->op == SELECT_WAIT) + { + if (!nb_locally_lockable_objs) + goto local_done; + + /* waitformultipleobjectsex with bWaitAll = FALSE must go in order */ + for (i = 0; i < nb_handles; ++i) + { + /* If we can't lock this one locally, we must do the server call */ + if (!objs[i] || !objs[i]->ops->trywait) + break; + + result = objs[i]->ops->trywait(objs[i]); + if (result == STATUS_SUCCESS) + { + TRACE_(ntdllsync)("Successful local wait any. obj = %p (h = %p)\n", objs[i], objs[i]->h); + + ret = STATUS_SUCCESS; + wait_object = i; + break; + } + else if (result == STATUS_WAS_LOCKED) + continue; + else + ret = result; + } + } + + else /* select_op->op == SELECT_WAIT_ALL (bWaitAll = TRUE)*/ + { + ULONGLONG locked_objs = 0; + assert(MAXIMUM_WAIT_OBJECTS <= sizeof(locked_objs) * 8); + + if (nb_locally_lockable_objs != nb_handles) + goto local_done; + + result = 0; + + /* try to lock all objects */ + for (i = 0; i < nb_handles; ++i) + { + result = objs[i]->ops->trywait(objs[i]); + + if (result == STATUS_WAS_LOCKED) /* normal trywait failure */ + break; + else if (result != STATUS_POSSIBLE_DEADLOCK) + { + WARN("Possible deadlock in thread 0x%x, waiting on handle %p\n", GetCurrentThreadId(), objs[i]->h); + break; + } + else if (result != STATUS_SUCCESS) + { + ERR("object (handle = %p) trywait returned 0x%x\n", objs[i]->h, result); + break; + } + + /* keep track of the objects we've locked, max 64 */ + locked_objs |= 1LL << i; + TRACE_(ntdllsync)("locked object %zd/%zd\n", i, nb_handles); + } + + /* This is a little redundant, if result == STATUS_SUCCESS, then i should also be == nb_handles */ + if (result == STATUS_SUCCESS && i == nb_handles) + { + TRACE_(ntdllsync)("Successful local wait all. count = %zu\n", nb_handles); + ret = STATUS_SUCCESS; + } + /* if not successful then roll back locks */ + else + { + TRACE_(ntdllsync)("rolling back...\n"); + for (; i >= 0; --i) + { + TRACE_(ntdllsync)("rolling back %zd/%zd\n", i, nb_handles); + if (locked_objs & (1 << i)) + objs[i]->ops->trywait_undo(objs[i]); + } + + if (result != STATUS_WAS_LOCKED) + return result; + } + } + +local_done: + + /* release any objects we've grabbed */ + for (i = 0; i < nb_handles; ++i) + if (objs[i]) + ntdll_object_release(objs[i]); + + switch (ret) + { + case STATUS_SUCCESS: + return WAIT_OBJECT_0 + wait_object; + + default: + /* fall through to server call */ + break; + } + } +TRACE_(ntdllsync)("Doing server call.\n"); + +#endif /* ENABLE_POSIX_SYNC */ + for (;;) { SERVER_START_REQ( select )
Hi,
On 09/11/2015 01:24 AM, Daniel Santos wrote:
I've had to split up the 4th patch because my mail server wouldn't send a 20k message (weird), so I'm just resending the whole set in hopes to avoid confusion.
Are you sure you're not a victim of this: http://lists-archives.com/git/846970-git-send-email-connection-closed.html
I needed that ugly git fix/hack to send my last patches.
Sorry for the late reply.
Regards, Florian
On 09/11/2015 07:34 AM, Florian Pelz wrote:
Hi,
On 09/11/2015 01:24 AM, Daniel Santos wrote:
I've had to split up the 4th patch because my mail server wouldn't send a 20k message (weird), so I'm just resending the whole set in hopes to avoid confusion.
Are you sure you're not a victim of this: http://lists-archives.com/git/846970-git-send-email-connection-closed.html
I needed that ugly git fix/hack to send my last patches.
Sorry for the late reply.
Regards, Florian
Wow, thanks! that is exactly what I'm getting.
Net::SMTP::SSL: Net::Cmd::datasend(): unexpected EOF on command channel: at /usr/libexec/git-core/git-send-email line 1320. [Net::SMTP::SSL] Connection closed at /usr/libexec/git-core/git-send-email line 1320.
Thanks!
This is the switch that turns on the patchset via the config.h macro ENABLE_HYBRID_SYNC. --- configure | 18 ++++++++++++++++++ configure.ac | 7 +++++++ include/config.h.in | 6 ++++++ 3 files changed, 31 insertions(+)
diff --git a/configure b/configure index 15a122b..23cabcb 100755 --- a/configure +++ b/configure @@ -800,6 +800,7 @@ enable_win16 enable_win64 enable_tests enable_maintainer_mode +enable_hybrid_sync with_alsa with_capi with_cms @@ -2155,6 +2156,8 @@ Optional Features: --disable-tests do not build the regression tests --enable-maintainer-mode enable maintainer-specific build rules + --enable-hybrid-sync enable hybrid of server-side and process local + synchronisation objects (experimental) --disable-largefile omit support for large files
Optional Packages: @@ -3260,6 +3263,11 @@ if test "${enable_maintainer_mode+set}" = set; then : enableval=$enable_maintainer_mode; fi
+# Check whether --enable-hybrid-sync was given. +if test "${enable_hybrid_sync+set}" = set; then : + enableval=$enable_hybrid_sync; +fi +
# Check whether --with-alsa was given. @@ -6700,6 +6708,7 @@ for ac_header in \ scsi/scsi.h \ scsi/scsi_ioctl.h \ scsi/sg.h \ + semaphore.h \ stdbool.h \ stdint.h \ stropts.h \ @@ -7057,6 +7066,15 @@ fi done
+if test "x$enable_hybrid_sync" = "xyes" +then + +cat >>confdefs.h <<_ACEOF +#define ENABLE_HYBRID_SYNC 1 +_ACEOF + +fi + for ac_header in linux/videodev.h linux/videodev2.h libv4l1.h do : as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh` diff --git a/configure.ac b/configure.ac index aec53f6..d55281c 100644 --- a/configure.ac +++ b/configure.ac @@ -32,6 +32,7 @@ AC_ARG_ENABLE(win16, AS_HELP_STRING([--disable-win16],[do not include Win16 supp AC_ARG_ENABLE(win64, AS_HELP_STRING([--enable-win64],[build a Win64 emulator on AMD64 (won't run Win32 binaries)])) AC_ARG_ENABLE(tests, AS_HELP_STRING([--disable-tests],[do not build the regression tests])) AC_ARG_ENABLE(maintainer-mode, AS_HELP_STRING([--enable-maintainer-mode],[enable maintainer-specific build rules])) +AC_ARG_ENABLE(hybrid-sync, AS_HELP_STRING([--enable-hybrid-sync],[enable hybrid of server-side and process local synchronisation objects (experimental)]))
AC_ARG_WITH(alsa, AS_HELP_STRING([--without-alsa],[do not use the Alsa sound support]), [if test "x$withval" = "xno"; then ac_cv_header_sys_asoundlib_h=no; ac_cv_header_alsa_asoundlib_h=no; fi]) @@ -462,6 +463,7 @@ AC_CHECK_HEADERS(\ scsi/scsi.h \ scsi/scsi_ioctl.h \ scsi/sg.h \ + semaphore.h \ stdbool.h \ stdint.h \ stropts.h \ @@ -650,6 +652,11 @@ AC_CHECK_HEADERS([pthread_np.h],,, #include <pthread.h> #endif])
+if test "x$enable_hybrid_sync" = "xyes" +then + AC_DEFINE_UNQUOTED(ENABLE_HYBRID_SYNC, 1, [Define to 1 to enable hybrid synchronization.]) +fi + AC_CHECK_HEADERS([linux/videodev.h linux/videodev2.h libv4l1.h],,, [#ifdef HAVE_SYS_TIME_H #include <sys/time.h> diff --git a/include/config.h.in b/include/config.h.in index eb61a94..4e18208 100644 --- a/include/config.h.in +++ b/include/config.h.in @@ -7,6 +7,9 @@ /* Define to a function attribute for Microsoft hotpatch assembly prefix. */ #undef DECLSPEC_HOTPATCH
+/* Define to 1 to enable hybrid synchronization. */ +#undef ENABLE_HYBRID_SYNC + /* Define to the file extension for executables. */ #undef EXEEXT
@@ -759,6 +762,9 @@ /* Define to 1 if you have the `select' function. */ #undef HAVE_SELECT
+/* Define to 1 if you have the <semaphore.h> header file. */ +#undef HAVE_SEMAPHORE_H + /* Define to 1 if you have the `sendmsg' function. */ #undef HAVE_SENDMSG
When the server creates or opens a semaphore, the key is passed to the client so that they may open the same semaphore locally. --- include/wine/server_protocol.h | 10 +++++++--- server/protocol.def | 5 +++++ server/request.h | 9 +++++++-- server/trace.c | 5 +++++ 4 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 2ba71e8..81c99d2 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -1308,7 +1308,8 @@ struct create_semaphore_reply { struct reply_header __header; obj_handle_t handle; - char __pad_12[4]; + unsigned int key; + client_ptr_t server_ptr; };
@@ -1352,7 +1353,10 @@ struct open_semaphore_reply { struct reply_header __header; obj_handle_t handle; - char __pad_12[4]; + unsigned int max; + unsigned int key; + char __pad_20[4]; + client_ptr_t server_ptr; };
@@ -6149,6 +6153,6 @@ union generic_reply struct terminate_job_reply terminate_job_reply; };
-#define SERVER_PROTOCOL_VERSION 487 +#define SERVER_PROTOCOL_VERSION 488
#endif /* __WINE_WINE_SERVER_PROTOCOL_H */ diff --git a/server/protocol.def b/server/protocol.def index c313006..38bcca6 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1103,6 +1103,8 @@ enum event_op { PULSE_EVENT, SET_EVENT, RESET_EVENT }; VARARG(objattr,object_attributes); /* object attributes */ @REPLY obj_handle_t handle; /* handle to the semaphore */ + unsigned int key; /* key to the native semaphore */ + client_ptr_t server_ptr; /* for debugging only */ @END
@@ -1129,6 +1131,9 @@ enum event_op { PULSE_EVENT, SET_EVENT, RESET_EVENT }; VARARG(name,unicode_str); /* object name */ @REPLY obj_handle_t handle; /* handle to the semaphore */ + unsigned int max; /* maximum count */ + unsigned int key; /* key to the native semaphore */ + client_ptr_t server_ptr; /* for debugging only */ @END
diff --git a/server/request.h b/server/request.h index 9405179..bfa4ef9 100644 --- a/server/request.h +++ b/server/request.h @@ -918,7 +918,9 @@ C_ASSERT( FIELD_OFFSET(struct create_semaphore_request, initial) == 20 ); C_ASSERT( FIELD_OFFSET(struct create_semaphore_request, max) == 24 ); C_ASSERT( sizeof(struct create_semaphore_request) == 32 ); C_ASSERT( FIELD_OFFSET(struct create_semaphore_reply, handle) == 8 ); -C_ASSERT( sizeof(struct create_semaphore_reply) == 16 ); +C_ASSERT( FIELD_OFFSET(struct create_semaphore_reply, key) == 12 ); +C_ASSERT( FIELD_OFFSET(struct create_semaphore_reply, server_ptr) == 16 ); +C_ASSERT( sizeof(struct create_semaphore_reply) == 24 ); C_ASSERT( FIELD_OFFSET(struct release_semaphore_request, handle) == 12 ); C_ASSERT( FIELD_OFFSET(struct release_semaphore_request, count) == 16 ); C_ASSERT( sizeof(struct release_semaphore_request) == 24 ); @@ -934,7 +936,10 @@ C_ASSERT( FIELD_OFFSET(struct open_semaphore_request, attributes) == 16 ); C_ASSERT( FIELD_OFFSET(struct open_semaphore_request, rootdir) == 20 ); C_ASSERT( sizeof(struct open_semaphore_request) == 24 ); C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, handle) == 8 ); -C_ASSERT( sizeof(struct open_semaphore_reply) == 16 ); +C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, max) == 12 ); +C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, key) == 16 ); +C_ASSERT( FIELD_OFFSET(struct open_semaphore_reply, server_ptr) == 24 ); +C_ASSERT( sizeof(struct open_semaphore_reply) == 32 ); C_ASSERT( FIELD_OFFSET(struct create_file_request, access) == 12 ); C_ASSERT( FIELD_OFFSET(struct create_file_request, attributes) == 16 ); C_ASSERT( FIELD_OFFSET(struct create_file_request, sharing) == 20 ); diff --git a/server/trace.c b/server/trace.c index 07aee80..fbaa426 100644 --- a/server/trace.c +++ b/server/trace.c @@ -1592,6 +1592,8 @@ static void dump_create_semaphore_request( const struct create_semaphore_request static void dump_create_semaphore_reply( const struct create_semaphore_reply *req ) { fprintf( stderr, " handle=%04x", req->handle ); + fprintf( stderr, ", key=%08x", req->key ); + dump_uint64( ", server_ptr=", &req->server_ptr ); }
static void dump_release_semaphore_request( const struct release_semaphore_request *req ) @@ -1627,6 +1629,9 @@ static void dump_open_semaphore_request( const struct open_semaphore_request *re static void dump_open_semaphore_reply( const struct open_semaphore_reply *req ) { fprintf( stderr, " handle=%04x", req->handle ); + fprintf( stderr, ", max=%08x", req->max ); + fprintf( stderr, ", key=%08x", req->key ); + dump_uint64( ", server_ptr=", &req->server_ptr ); }
static void dump_create_file_request( const struct create_file_request *req )
Changes server to use a posix semaphore instead of an int. This patch alone would be pointless, but creates a mechanism for client processes to interact with the same semaphores. --- server/Makefile.in | 2 +- server/object.h | 4 + server/semaphore.c | 272 +++++++++++++++++++++++++++++++++++++++++++++++------ server/thread.c | 54 ++++++++++- 4 files changed, 297 insertions(+), 35 deletions(-)
diff --git a/server/Makefile.in b/server/Makefile.in index 19a4fac..48a4522 100644 --- a/server/Makefile.in +++ b/server/Makefile.in @@ -1,4 +1,4 @@ -EXTRALIBS = $(POLL_LIBS) $(RT_LIBS) +EXTRALIBS = $(POLL_LIBS) $(RT_LIBS) $(PTHREAD_LIBS)
C_SRCS = \ async.c \ diff --git a/server/object.h b/server/object.h index b59811f..ead4c50 100644 --- a/server/object.h +++ b/server/object.h @@ -90,6 +90,10 @@ struct object_ops int (*close_handle)(struct object *,struct process *,obj_handle_t); /* destroy on refcount == 0 */ void (*destroy)(struct object *); + /* like signaled(), but tries to get the lock, returns 1 upon success */ + int (*trylock)(struct object *,struct wait_queue_entry *); + /* undo a previously sucessful call to trylock */ + void (*trylock_undo)(struct object *,struct wait_queue_entry *); };
struct object diff --git a/server/semaphore.c b/server/semaphore.c index d87325c..6900804 100644 --- a/server/semaphore.c +++ b/server/semaphore.c @@ -26,6 +26,21 @@ #include <stdlib.h> #include <stdarg.h>
+#ifdef ENABLE_HYBRID_SYNC +# include <fcntl.h> +# include <sys/stat.h> +# include <errno.h> +# ifdef HAVE_SEMAPHORE_H +# include <semaphore.h> +# endif +# ifdef HAVE_SYS_TYPES_H +# include <sys/types.h> +# endif +# ifdef HAVE_UNISTD_H +# include <unistd.h> +# endif +#endif + #include "ntstatus.h" #define WIN32_NO_STATUS #include "windef.h" @@ -36,10 +51,17 @@ #include "request.h" #include "security.h"
+#define NATIVE_SEMAPHORE_MAX_NAME (32) struct semaphore { struct object obj; /* object header */ +#ifdef ENABLE_HYBRID_SYNC + sem_t *p; + unsigned int key; + char name[NATIVE_SEMAPHORE_MAX_NAME]; +#else unsigned int count; /* current count */ +#endif /* ENABLE_HYBRID_SYNC */ unsigned int max; /* maximum possible count */ };
@@ -49,6 +71,14 @@ static int semaphore_signaled( struct object *obj, struct wait_queue_entry *entr static void semaphore_satisfied( struct object *obj, struct wait_queue_entry *entry ); static unsigned int semaphore_map_access( struct object *obj, unsigned int access ); static int semaphore_signal( struct object *obj, unsigned int access ); +static void semaphore_new( struct semaphore *sem, unsigned int initial ); +static unsigned int semaphore_get_value( struct semaphore *sem ); + +#ifdef ENABLE_HYBRID_SYNC +static void semaphore_destroy( struct object *obj ); +static int semaphore_trylock( struct object *obj, struct wait_queue_entry *entry ); +static void semaphore_trylock_undo( struct object *obj, struct wait_queue_entry *entry ); +#endif /* ENABLE_HYBRID_SYNC */
static const struct object_ops semaphore_ops = { @@ -67,35 +97,32 @@ static const struct object_ops semaphore_ops = no_lookup_name, /* lookup_name */ no_open_file, /* open_file */ no_close_handle, /* close_handle */ - no_destroy /* destroy */ +#ifdef ENABLE_HYBRID_SYNC + semaphore_destroy, /* destroy */ + semaphore_trylock, /* trylock */ + semaphore_trylock_undo /* trylock_undo */ +#else + no_destroy, /* destroy */ + NULL, /* trylock */ + NULL /* trylock_undo */ +#endif };
+#ifndef ENABLE_HYBRID_SYNC
-static struct semaphore *create_semaphore( struct directory *root, const struct unicode_str *name, - unsigned int attr, unsigned int initial, unsigned int max, - const struct security_descriptor *sd ) +static inline void semaphore_new( struct semaphore *sem, unsigned int initial ) { - struct semaphore *sem; + sem->count = initial; +}
- if (!max || (initial > max)) - { - set_error( STATUS_INVALID_PARAMETER ); - return NULL; - } - if ((sem = create_named_object_dir( root, name, attr, &semaphore_ops ))) - { - if (get_error() != STATUS_OBJECT_NAME_EXISTS) - { - /* initialize it if it didn't already exist */ - sem->count = initial; - sem->max = max; - if (sd) default_set_sd( &sem->obj, sd, OWNER_SECURITY_INFORMATION| - GROUP_SECURITY_INFORMATION| - DACL_SECURITY_INFORMATION| - SACL_SECURITY_INFORMATION ); - } - } - return sem; +static inline unsigned int semaphore_get_value( struct semaphore *sem ) +{ + return sem->count; +} + +static inline void semaphore_down( struct semaphore *sem ) +{ + --sem->count; }
static int release_semaphore( struct semaphore *sem, unsigned int count, @@ -119,12 +146,188 @@ static int release_semaphore( struct semaphore *sem, unsigned int count, } return 1; } +#else /* ENABLE_HYBRID_SYNC */ + +static DWORD next_semaphore_id = 0; + +static void semaphore_new( struct semaphore *sem, unsigned int initial ) +{ + /* imperfect, but good enough for now */ + int tries; + for (tries = 0; tries < 0x10000; ++tries) + { + DWORD pid = (DWORD)getpid(); + DWORD reverse_pid = 0; + DWORD bit; + + for (bit = (1 << (sizeof(DWORD) * 8 - 1)); bit && pid; bit >>= 1, pid >>= 1) + { + if (pid & 1) + reverse_pid |= bit; + } + + sem->key = reverse_pid ^ next_semaphore_id++; + snprintf(sem->name, sizeof(sem->name) - 1, "/wine-sem-%08x", sem->key); + sem->p = sem_open(sem->name, O_CREAT | O_EXCL, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP, initial); + + if (sem->p != SEM_FAILED) + { + /* fprintf(stderr, "semaphore_new: initial = %u, sem = %p, sem->p = %p, sem->name = %s\n", + initial, sem, sem->p, sem->name); */ + return; + } + + switch (errno) + { + case EACCES: + case EEXIST: + continue; + + case EINVAL: + set_error( STATUS_INVALID_PARAMETER ); + break; + case EMFILE: + case ENFILE: + set_error( STATUS_TOO_MANY_OPENED_FILES ); + break; + case ENOMEM: + set_error( STATUS_NO_MEMORY ); + break; + default: + break; + } + perror("sem_open"); + assert(0); + exit(1); + } + fprintf(stderr, "failed to create unique key for semaphore\n"); + assert(0); +} + +static unsigned int semaphore_get_value( struct semaphore *sem ) +{ + int ret; + + if (sem_getvalue(sem->p, &ret) == -1) + { + perror("sem_getvalue"); + exit(1); + + return STATUS_INVALID_HANDLE; + } + + return (unsigned int)ret; +} + +static void semaphore_destroy( struct object *obj ) +{ + struct semaphore *sem = (struct semaphore *)obj; + assert( obj->ops == &semaphore_ops ); + sem_destroy(sem->p); +} + +static int semaphore_trylock( struct object *obj, struct wait_queue_entry *entry ) +{ + struct semaphore *sem = (struct semaphore *)obj; + assert( obj->ops == &semaphore_ops ); + if (sem_trywait(sem->p) == -1) + { + switch(errno) { + case EAGAIN: + return 0; + + default: + perror("sem_trywait"); + exit(1); + } + } + return 1; +} + +static void semaphore_trylock_undo( struct object *obj, struct wait_queue_entry *entry ) +{ + struct semaphore *sem = (struct semaphore *)obj; + assert( obj->ops == &semaphore_ops ); + if (sem_post(sem->p) == -1) + { + perror("sem_post"); + exit(1); + } + +} + +static int release_semaphore( struct semaphore *sem, unsigned int count, + unsigned int *prev ) +{ + unsigned int cur_count; + + cur_count = semaphore_get_value(sem); + + if (prev) *prev = cur_count; + if (cur_count + count < cur_count || cur_count + count > sem->max) + { + set_error( STATUS_SEMAPHORE_LIMIT_EXCEEDED ); + return 0; + } + else + { + while(count--) + { + if (sem_post(sem->p) == -1) + { + /* FIXME: no atomic way to increase more than once */ + perror("sem_post"); + } + } + + /* there cannot be any thread to wake up if the count is != 0 */ + if (!cur_count) + wake_up( &sem->obj, count ); + } + return 1; +} + +#endif /* ENABLE_HYBRID_SYNC */ + +static struct semaphore *create_semaphore( struct directory *root, const struct unicode_str *name, + unsigned int attr, unsigned int initial, unsigned int max, + const struct security_descriptor *sd ) +{ + struct semaphore *sem; + + if (!max || (initial > max)) + { + set_error( STATUS_INVALID_PARAMETER ); + return NULL; + } + if ((sem = create_named_object_dir( root, name, attr, &semaphore_ops ))) + { + if (get_error() != STATUS_OBJECT_NAME_EXISTS) + { + /* initialize it if it didn't already exist */ + semaphore_new(sem, initial); + sem->max = max; + if (sd) default_set_sd( &sem->obj, sd, OWNER_SECURITY_INFORMATION| + GROUP_SECURITY_INFORMATION| + DACL_SECURITY_INFORMATION| + SACL_SECURITY_INFORMATION ); + } +#ifdef ENABLE_HYBRID_SYNC + else + { + assert(sem->p); + assert(sem->name[0]); + } +#endif + } + return sem; +}
static void semaphore_dump( struct object *obj, int verbose ) { struct semaphore *sem = (struct semaphore *)obj; assert( obj->ops == &semaphore_ops ); - fprintf( stderr, "Semaphore count=%d max=%d ", sem->count, sem->max ); + fprintf( stderr, "Semaphore count=%d max=%d ", semaphore_get_value(sem), sem->max ); dump_object_name( &sem->obj ); fputc( '\n', stderr ); } @@ -140,15 +343,17 @@ static int semaphore_signaled( struct object *obj, struct wait_queue_entry *entr { struct semaphore *sem = (struct semaphore *)obj; assert( obj->ops == &semaphore_ops ); - return (sem->count > 0); + return (semaphore_get_value(sem) > 0); }
static void semaphore_satisfied( struct object *obj, struct wait_queue_entry *entry ) { +#ifndef ENABLE_HYBRID_SYNC struct semaphore *sem = (struct semaphore *)obj; assert( obj->ops == &semaphore_ops ); - assert( sem->count ); - sem->count--; + assert( semaphore_get_value(sem) ); + semaphore_down(sem); +#endif }
static unsigned int semaphore_map_access( struct object *obj, unsigned int access ) @@ -199,6 +404,10 @@ DECL_HANDLER(create_semaphore) reply->handle = alloc_handle( current->process, sem, req->access, req->attributes ); else reply->handle = alloc_handle_no_access_check( current->process, sem, req->access, req->attributes ); +#ifdef ENABLE_HYBRID_SYNC + reply->key = sem->key; +#endif + reply->server_ptr = (unsigned long)sem; release_object( sem ); }
@@ -219,6 +428,11 @@ DECL_HANDLER(open_semaphore) if ((sem = open_object_dir( root, &name, req->attributes, &semaphore_ops ))) { reply->handle = alloc_handle( current->process, &sem->obj, req->access, req->attributes ); +#ifdef ENABLE_HYBRID_SYNC + reply->key = sem->key; +#endif + reply->max = sem->max; + reply->server_ptr = (unsigned long)sem; release_object( sem ); }
@@ -246,7 +460,7 @@ DECL_HANDLER(query_semaphore) if ((sem = (struct semaphore *)get_handle_obj( current->process, req->handle, SEMAPHORE_QUERY_STATE, &semaphore_ops ))) { - reply->current = sem->count; + reply->current = semaphore_get_value(sem); reply->max = sem->max; release_object( sem ); } diff --git a/server/thread.c b/server/thread.c index 6383000..bb21b93 100644 --- a/server/thread.c +++ b/server/thread.c @@ -653,6 +653,12 @@ static int check_wait( struct thread *thread ) int i; struct thread_wait *wait = thread->wait; struct wait_queue_entry *entry; + const int hybrid_sync_enabled = +#ifdef ENABLE_HYBRID_SYNC + 1; +#else + 0; +#endif
assert( wait );
@@ -662,24 +668,62 @@ static int check_wait( struct thread *thread ) /* Suspended threads may not acquire locks, but they can run system APCs */ if (thread->process->suspend + thread->suspend > 0) return -1;
- if (wait->select == SELECT_WAIT_ALL) + if (wait->select == SELECT_WAIT_ALL) /* bWaitAll == TRUE */ { int not_ok = 0; - /* Note: we must check them all anyway, as some objects may - * want to do something when signaled, even if others are not */ + int have_hybrid_objects = 0; + ULONGLONG undo_list = 0; + + assert(MAXIMUM_WAIT_OBJECTS <= sizeof(ULONGLONG) * 8); + /* Note: we must check them all anyway, as some objects (msg queues) + * may want to do something when signaled, even if others are not */ for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) + { not_ok |= !entry->obj->ops->signaled( entry->obj, entry ); + have_hybrid_objects |= hybrid_sync_enabled && entry->obj->ops->trylock; + } if (not_ok) goto other_checks; + + if (!have_hybrid_objects) + goto do_satisfied; + + /* all objects signaled, try to acquire locks on hybrid objects */ + for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) + { + if (!entry->obj->ops->trylock) + continue; + + /* it's still possible for failure because one of the hybrid objects + * may have since been locked */ + if (entry->obj->ops->trylock( entry->obj, entry )) + undo_list |= 1ULL << i; + else + { + /* if a lock cannot be acquired, then undo previously acquired locks */ + for (--i, --entry; i >= 0; --i, --entry) + { + if (undo_list & (1ULL << i)) + entry->obj->ops->trylock_undo( entry->obj, entry ); + } + goto other_checks; + } + } + +do_satisfied: /* Wait satisfied: tell it to all objects */ for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) entry->obj->ops->satisfied( entry->obj, entry ); return wait->abandoned ? STATUS_ABANDONED_WAIT_0 : STATUS_WAIT_0; } - else + else /* bWaitAll == FALSE */ { for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) { - if (!entry->obj->ops->signaled( entry->obj, entry )) continue; + int signaled = hybrid_sync_enabled && entry->obj->ops->trylock + ? entry->obj->ops->trylock( entry->obj, entry ) + : entry->obj->ops->signaled( entry->obj, entry ); + + if (!signaled) continue; /* Wait satisfied: tell it to the object */ entry->obj->ops->satisfied( entry->obj, entry ); if (wait->abandoned) i += STATUS_ABANDONED_WAIT_0;
Adds a facility for ntdll to track handles locally and attach information to them, allowing for some operations to be performed directly by the client process. --- dlls/ntdll/loader.c | 1 + dlls/ntdll/ntdll_misc.h | 62 ++++++ dlls/ntdll/om.c | 568 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 631 insertions(+)
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 8a43310..0c779e2 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -3258,6 +3258,7 @@ void __wine_process_init(void) umask( FILE_umask );
load_global_options(); + ntdll_object_db_init();
/* setup the load callback and create ntdll modref */ wine_dll_set_callback( load_builtin_callback ); diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h index cbd19db..f0b702e 100644 --- a/dlls/ntdll/ntdll_misc.h +++ b/dlls/ntdll/ntdll_misc.h @@ -28,6 +28,8 @@ #include "winnt.h" #include "winternl.h" #include "wine/server.h" +#include "wine/rbtree.h" +#include "wine/list.h"
#define MAX_NT_PATH_LENGTH 277
@@ -268,4 +270,64 @@ extern HANDLE keyed_event DECLSPEC_HIDDEN;
NTSTATUS WINAPI RtlHashUnicodeString(PCUNICODE_STRING,BOOLEAN,ULONG,ULONG*);
+enum ntdll_obj_type { + NTDLL_OBJ_TYPE_ASYNC, /* async I/O */ + NTDLL_OBJ_TYPE_EVENT, /* event */ + NTDLL_OBJ_TYPE_COMPLETION, /* IO completion ports */ + NTDLL_OBJ_TYPE_FILE, /* file */ + NTDLL_OBJ_TYPE_MAILSLOT, /* +2 chain mail slot */ + NTDLL_OBJ_TYPE_MUTEX, /* mutex */ + NTDLL_OBJ_TYPE_MSG_QUEUE, /* message queue */ + NTDLL_OBJ_TYPE_PROCESS, /* process */ + NTDLL_OBJ_TYPE_SEMAPHORE, /* semaphore */ + NTDLL_OBJ_TYPE_THREAD, /* thread */ + NTDLL_OBJ_TYPE_WAITABLE_TIMER, /* waitable timer */ + + NTDLL_OBJ_TYPE_MAX +}; + +struct ntdll_object; + +struct ntdll_object_ops { + /* wait on the object (TODO) */ + NTSTATUS (*wait) (struct ntdll_object *obj, const LARGE_INTEGER *timeout); + /* returns true if the object is currently signaled */ + BOOL (*signaled) (struct ntdll_object *obj); + /* attempt to obtain lock on object, return STATUS_SUCCESS or STATUS_WAS_LOCKED upon failure. */ + NTSTATUS (*trywait) (struct ntdll_object *obj); + /* called to release a lock previous obtained by trywait() */ + NTSTATUS (*trywait_undo)(struct ntdll_object *obj); + /* close the client-side object */ + void (*close) (struct ntdll_object *obj); + /* Dump a description of the object to the supplied buffer. The implementation should + * generally call ntdll_object_dump_base() to describe it's struct ntdll_object member.*/ + void (*dump) (const struct ntdll_object *obj, char **start, const char *const end); +}; + +/* process-local data for ntdll objects + * + * currently, fields ops - server_ptr should sit on one cache line in all cases, + * list_entry used infrequently */ +struct ntdll_object { + struct ntdll_object_ops *ops; /* ops */ + int refcount; /* reference count */ + HANDLE h; /* handle to the object (may be more than one handle tot he same server-side obj) */ + enum ntdll_obj_type type_id; /* redundant? maybe but keep for debugging for now */ + struct wine_rb_entry tree_entry; /* red-black tree node */ + ULONG_PTR server_ptr; /* for debugging only */ + struct list list_entry; +}; + +extern NTSTATUS ntdll_object_db_init(void); +extern struct ntdll_object *ntdll_object_new(const HANDLE h, size_t size, enum ntdll_obj_type type_id, struct ntdll_object_ops *ops); +extern struct ntdll_object *ntdll_object_grab(struct ntdll_object *obj); +extern void ntdll_object_release(struct ntdll_object *obj); +extern void ntdll_object_dump_base(const struct ntdll_object *obj, char **start, const char *const end); +extern const char *ntdll_object_dump(const struct ntdll_object *obj); +extern int ntdll_handle_add(struct ntdll_object *obj); +extern void ntdll_handle_remove(const HANDLE h); +extern struct ntdll_object *ntdll_handle_find(const HANDLE h); + +extern void ntdll_server_notify_lock_release(void); + #endif diff --git a/dlls/ntdll/om.c b/dlls/ntdll/om.c index 3fadba7..3ccce4c 100644 --- a/dlls/ntdll/om.c +++ b/dlls/ntdll/om.c @@ -20,6 +20,7 @@ */
#include "config.h" +#include "wine/port.h"
#include <stdarg.h> #include <stdlib.h> @@ -30,6 +31,9 @@ #ifdef HAVE_UNISTD_H # include <unistd.h> #endif +#include <assert.h> +#include <limits.h> +#include <stdio.h>
#include "ntstatus.h" #define WIN32_NO_STATUS @@ -40,6 +44,567 @@ #include "wine/server.h"
WINE_DEFAULT_DEBUG_CHANNEL(ntdll); +WINE_DECLARE_DEBUG_CHANNEL(ntdll_obj); + + +/* + * Process-local object information management + * + * Theory of Operation + * =================== + * Information about some ntdll objects are cached locally and used to facilitate + * IPC operations natively, keeping the server informed about anything important + * via asynchronous messages, preventing the need for the big context switch. This + * works for releasing locks and attempting to wait on one or more supported objects. + * All other WaitFor* calls are routed to the server. + * + * Lifecycle of a struct ntdll_object object + * + * refcount Operation + * (no object) Execute a server call that creates an object & returns a handle. + * 1 Call ntdll_object_new() passing handle to allocate a new struct ntdll_object. + * * allocates & zeros struct ntdll_object + * * adds to objects.list (double-linked list) + * 1 Initialize derived-type data members + * 2 Call ntdll_handle_add() to index handle in handles.tree (red-black tree) + * 1 Call ntdll_object_release() when done with the object. + * .... + * (no object) We receive an API call that we might be able to manage client-side + * 2 Call ntdll_handle_find() on the handle and get the object + * 2 Perform whatever we need to do locally + * 1 Call ntdll_object_release() when done with the object. + * ... + * (no object) We receive a call to NtClose (CloseHandle) + * 2 Call ntdll_handle_find() on the handle and get the object + * 1 Call ntdll_handle_remove() to remove it from handles.tree. + * 0 Call ntdll_object_release() + * * object is removed from list + * * refcount reaches zero and destructor ops->close() is called + * * memory is freed + * + * Because we're doing this in the multi-threaded environment of the client process instead of + * the safety of the single-threaded server, things won't always go as above. It is possible + * for a call to NtClose to be received while another thread is still using the object, so + * that the object returned by Call ntdll_handle_find() may have a refcount of 3 or more. + * However, the object should be immediately delisted, so we can have many live objects + * with the same handle, but only one of those handles (the one in the tree) is the living + * one. + * + * TODO: Make sure that UB is acceptable for when a program tries to use a stale handle, or + * should we zero the handle when it's removed from the tree (NtClose returns). + */ + + +/* FIXME: portability */ +static __thread char *tls_debug_buffer = NULL; +#define NTDLL_DEBUG_BUFFER_SIZE 0x400 + +static const char * const obj_type_desc[NTDLL_OBJ_TYPE_MAX] = { + "async I/O", /* NTDLL_OBJ_TYPE_ASYNC */ + "event", /* NTDLL_OBJ_TYPE_EVENT */ + "I/O completion ports", /* NTDLL_OBJ_TYPE_COMPLETION */ + "file", /* NTDLL_OBJ_TYPE_FILE */ + "mail slot", /* NTDLL_OBJ_TYPE_MAILSLOT */ + "mutex", /* NTDLL_OBJ_TYPE_MUTEX */ + "message queue", /* NTDLL_OBJ_TYPE_MSG_QUEUE */ + "process", /* NTDLL_OBJ_TYPE_PROCESS */ + "semaphore", /* NTDLL_OBJ_TYPE_SEMAPHORE */ + "thread", /* NTDLL_OBJ_TYPE_THREAD */ + "waitable timer", /* NTDLL_OBJ_TYPE_WAITABLE_TIMER */ +}; + +/* List containing all process-locally known objects */ +static struct { + struct list list; /* list of all struct ntdll_object ojbects */ + pthread_mutex_t mutex; /* lock for modifying list */ +} objects = { + {NULL, NULL}, + PTHREAD_MUTEX_INITIALIZER, /* non-recursive fast lock */ +}; + +/************************************************************************* + * ntdll_object_new + * + * Allocates and initializes a new struct ntdll_object object. + * + * PARAMS + * h [I] Handle of the object. + * size [I] Number of bytes to allocate. + * type_id [I] Integral type of the object + * ops [I] Pointer to function table. + * + * RETURNS + * Success: A pointer to the new object. + * Failure: Null, indicating a low memory condition. + * + * NOTES + * Allocates and initialises a struct ntdll_object object and adds it to + * process-global list. Initial refcount is 1. After the derived-type + * initialises its members the object should be added to the handle database + * by calling ntdll_handle_add(), which will increase its refcount to 2. Once + * this is done, the object should be released with ntdll_object_release(). + * If you release it before that, the object will auto-destruct. + * + * LOCKING + * objects.mutex + */ +struct ntdll_object *ntdll_object_new(const HANDLE h, size_t size, enum ntdll_obj_type type_id, struct ntdll_object_ops *ops) +{ + struct ntdll_object *obj; + + assert(size >= sizeof(*obj)); + assert(type_id < NTDLL_OBJ_TYPE_MAX); + + TRACE_(ntdll_obj)("%p, %zu, %u (%s), %p\n", h, size, type_id, obj_type_desc[type_id], ops); + + obj = RtlAllocateHeap(GetProcessHeap(), 0, size); + if (obj) + { + memset(obj, 0, size); + obj->h = h; + obj->refcount = 1; + obj->type_id = type_id; + obj->ops = ops; + obj->server_ptr = 0; + pthread_mutex_lock(&objects.mutex); + list_add_tail(&objects.list, &obj->list_entry); + pthread_mutex_unlock(&objects.mutex); + } else + ERR("Failed to alloc %zu bytes\n", sizeof(*obj)); + + return obj; +} + +/************************************************************************* + * ntdll_object_grab + * + * Increases refcount by one. + * + * PARAMS + * obj [I] The object. + */ +struct ntdll_object *ntdll_object_grab(struct ntdll_object *obj) +{ + int refcount = interlocked_xchg_add(&obj->refcount, 1); + assert(refcount < INT_MAX && refcount > 0); + return obj; +} + +/************************************************************************* + * ntdll_object_release + * + * Decrease refcount by one, possibly destroying the object + * + * PARAMS + * obj [I] The object. + * + * NOTES + * While using an object, it is possible for another thread to call + * CloseHandle(), removing it from the handle database (and decreasing its + * refcount). Therefore you should not use the pointer after a call to + * ntdll_object_release() returns. + * + * LOCKING + * objects.mutex + */ +void ntdll_object_release(struct ntdll_object *obj) +{ + int refcount = interlocked_xchg_add(&obj->refcount, -1); + assert(refcount > 0); + + if (!refcount) + { + /* TODO: check for any more race conditions */ + TRACE_(ntdll_obj)("Removing & freeing object %s\n", ntdll_object_dump(obj)); + pthread_mutex_lock(&objects.mutex); + list_remove(&obj->list_entry); + pthread_mutex_unlock(&objects.mutex); + if (obj->ops->close) + obj->ops->close(obj); +#if defined(DEBUG) || defined(DEBUG_OBJECTS) + memset(obj, 0xaa, sizeof(struct ntdll_object)); +#endif + RtlFreeHeap(GetProcessHeap(), 0, obj); + } +} + +/************************************************************************* + * ntdll_object_dump_base + * + * Called by derived-class to dump a text description of base class members. + * + * PARAMS + * obj [I] The object to dump + * start [IO] Pointer to a pointer to the next write position. + * end [I] Pointer to one byte past the end of the buffer. + * + * NOTES + * Called by derived class to dump struct ntdll_object to a string buffer. + * The pointer that start points to will be updated to the new end of the + * text so that subsequent writes may begin there. + */ +void ntdll_object_dump_base(const struct ntdll_object *obj, char **start, const char *const end) +{ + int count; + + assert(start && *start && end); + assert(end > *start); + + if (!obj) + count = snprintf(*start, end - *start, "(NULL)"); + else + { + assert(obj->type_id < NTDLL_OBJ_TYPE_MAX); + + count = snprintf(*start, end - *start, + "%p {" + "ops = %p, " + "refcount = %u, " + "h = %p, " + "type_id = %u (%s), " + "tree_entry = {left = %p, right = %p, flags = 0x%x}, " + "server_ptr = %lx, " + "list_entry = {next = %p, prev = %p}" + "}", + obj, + obj->ops, + obj->refcount, + obj->h, + obj->type_id, obj_type_desc[obj->type_id], + obj->tree_entry.left, obj->tree_entry.right, obj->tree_entry.flags, + obj->server_ptr, + obj->list_entry.next, obj->list_entry.prev); + } + + if (count < 0) + { + perror("snprintf"); + return; + } + + *start += count; +} + +/* return a thread-local buffer for dumping object descriptions to */ +static void *ntdll_object_get_debug_buffer(size_t *size) +{ + /* FIXME: free this on thread exit (not sure where that is hooked) */ + if (!tls_debug_buffer) + tls_debug_buffer = RtlAllocateHeap(GetProcessHeap(), 0, NTDLL_DEBUG_BUFFER_SIZE); + + if (size) + *size = tls_debug_buffer ? NTDLL_DEBUG_BUFFER_SIZE : 0; + return tls_debug_buffer; +} + +/************************************************************************* + * ntdll_object_dump + * + * Dumps a text description of the object to a thread-local buffer. + * + * PARAMS + * obj [I] The object to dump + * + * RETURNS + * A pointer to a text description of the object. + * + * NOTES + * Because this function uses a thread-local buffer, you must use the result + * before calling it again. For instance, this code will not work: + * + * ERR("%s will look the same as %s\n", ntdll_object_dump(o1), ntdll_object_dump(02)); + */ +const char *ntdll_object_dump(const struct ntdll_object *obj) +{ + char *start; + const char *ret; + size_t size; + + if (!(start = ntdll_object_get_debug_buffer(&size))) + return NULL; + + ret = start; + if (!obj) + snprintf(start, size, "(NULL)"); + else if (obj->ops->dump) + obj->ops->dump(obj, &start, start + size); + else + ntdll_object_dump_base(obj, &start, start + size); + + return ret; +} + +/* red-black tree functions for handle table */ +static inline void *ntdll_object_rb_alloc(size_t size) +{ + return RtlAllocateHeap(GetProcessHeap(), 0, size); +} + +static inline void *ntdll_object_rb_realloc(void *ptr, size_t size) +{ + return RtlReAllocateHeap(GetProcessHeap(), 0, ptr, size); +} + +static inline void ntdll_object_rb_free(void *ptr) +{ + RtlFreeHeap(GetProcessHeap(), 0, ptr); +} + +static inline int ntdll_object_handle_compare(const void *key, const struct wine_rb_entry *entry) +{ + const HANDLE *_a = key; + const HANDLE *_b = &WINE_RB_ENTRY_VALUE(entry, const struct ntdll_object, tree_entry)->h; + + return *_a > *_b ? 1 : (*_a < *_b ? -1 : 0); +} + +static const struct wine_rb_functions obj_handles_rb_ops = +{ + ntdll_object_rb_alloc, + ntdll_object_rb_realloc, + ntdll_object_rb_free, + ntdll_object_handle_compare, +}; + +/* Tree mapping all process-locally known kernel handles to a struct ntdll_object */ +static struct { + struct wine_rb_tree tree; + pthread_mutex_t mutex; +} handles = { + { &obj_handles_rb_ops, NULL, {NULL, 0, 0}}, /* static initializer to aid -findirect-inline */ + PTHREAD_MUTEX_INITIALIZER /* non-recursive fast lock */ +}; + +/************************************************************************* + * ntdll_handle_add + * + * Adds the object to the ntdll handle database + * + * PARAMS + * obj [I] The object. + * + * NOTES + * Adds the object to the handle database, increasing its reference count to + * account for it being referenced by the handle tree. + * + * LOCKING + * handles.mutex + */ +__attribute__((noinline, flatten)) +int ntdll_handle_add(struct ntdll_object *obj) +{ + int ret = 0; + + TRACE_(ntdll_obj)("%p\n", obj); + +try_again: + pthread_mutex_lock(&handles.mutex); + ret = wine_rb_put(&handles.tree, &obj->h, &obj->tree_entry); + + /* increase the ref count */ + if (!ret) + ntdll_object_grab(obj); + pthread_mutex_unlock(&handles.mutex); + + if (ret) + { + struct ntdll_object *existing = ntdll_handle_find(obj->h); + + /* bug if we get here -- this should be resolved now, but lets leave + * this here for now to be safe */ + ERR("Failed to insert handle %p.\n", obj->h); + if (existing) + { + ERR("Existing object: %s\n", ntdll_object_dump(existing)); + ntdll_object_release(existing); + } + else + ERR("No other object found with that handle...\n"); + ERR("New object: %s\n", ntdll_object_dump(obj)); + + assert(0); + + /* should probably just exit(1) here */ + ntdll_handle_remove(obj->h); + goto try_again; + } + + TRACE("Added obj %s\n", ntdll_object_dump(obj)); + + return ret; +} + +/************************************************************************* + * ntdll_handle_remove + * + * Remove an object from the handle database. + * + * PARAMS + * h [I] A handle to remove. + * + * NOTES + * If no object with the specified handle is found then nothing is done. + * + * LOCKING + * handles.mutex + */ +__attribute__((noinline)) +void ntdll_handle_remove(const HANDLE h) +{ + struct ntdll_object *obj = ntdll_handle_find(h); + + if (!obj) + return; + + TRACE_(ntdll_obj)("h = %p) obj = %s\n", h, ntdll_object_dump(obj)); + + pthread_mutex_lock(&handles.mutex); + /* FIXME: a more efficient wine_rb_remove_by_entry would be nice here */ + wine_rb_remove(&handles.tree, &h); + pthread_mutex_unlock(&handles.mutex); + + ntdll_object_release(obj); /* once for removing it from the tree */ + ntdll_object_release(obj); /* once for the call to ntdll_handle_find() */ +} + +/************************************************************************* + * ntdll_handle_find + * + * Searches the process-local handle database for the specified object. + * + * PARAMS + * h [I] Handle of an object. + * + * NOTES + * If an object with a matching handle is found then the object's refcount is + * incremented and a pointer to the object is returned. It is then the + * responsibility of the caller to release the object when done. + * + * RETURNS + * Success: A pointer to the object. + * Failure: NULL + * + * LOCKING + * handles.mutex + */ +struct ntdll_object *ntdll_handle_find(const HANDLE h) +{ + struct wine_rb_entry *entry; + struct ntdll_object *ret = NULL; + + pthread_mutex_lock(&handles.mutex); + entry = wine_rb_get(&handles.tree, &h); + + if (entry) + { + ret = WINE_RB_ENTRY_VALUE(entry, struct ntdll_object, tree_entry); + ntdll_object_grab(ret); + } + pthread_mutex_unlock(&handles.mutex); + + //TRACE_(ntdll_obj)("(%p) result: %p\n", h, ret); + + return ret; +} + + +/* callback for ntdll_objects_cleanup() + * + * LOCKING + * Locks objects.mutex (via ntdll_object_release()) + * Called when handles.mutex already locked + */ + +static void ntdll_objects_cleanup_cb(struct wine_rb_entry *entry, void *context) +{ + struct ntdll_object *obj = WINE_RB_ENTRY_VALUE(entry, struct ntdll_object, tree_entry); + size_t *leaked_handles = (size_t *)context; + + ++(*leaked_handles); + + ERR_(ntdll_obj)("Leaked object handle: %s\n", ntdll_object_dump(obj)); + + /* handles.mutex already locked */ + wine_rb_remove(&handles.tree, &obj->h); + + ntdll_object_release(obj); +} + +/* atexit() cleanup + * + * Locking: + * Locks handles.mutex --> then objects.mutex + */ +static void ntdll_objects_cleanup(void) +{ + size_t leaked_handles = 0; + size_t leaked_objects = 0; + + TRACE_(ntdll_obj)("\n"); + pthread_mutex_lock(&handles.mutex); + wine_rb_for_each_entry(&handles.tree, ntdll_objects_cleanup_cb, &leaked_handles); + pthread_mutex_unlock(&handles.mutex); + + pthread_mutex_lock(&objects.mutex); + leaked_objects = list_count(&objects.list); + if (leaked_objects) + { + struct ntdll_object *obj; + //struct list *i; + LIST_FOR_EACH_ENTRY( obj, &objects.list, struct ntdll_object , list_entry ) + { + ERR_(ntdll_obj)("Leaked object: %s\n", ntdll_object_dump(obj)); + } + } + pthread_mutex_unlock(&objects.mutex); + + if (leaked_handles || leaked_objects) + ERR_(ntdll_obj)("*** %zu leaked handles found, %zu leaked objects remain.\n", + leaked_handles, leaked_objects); + +} + +/************************************************************************* + * ntdll_object_db_init + * + * Initialize objects list and handles tree + * + * NOTES + * Called via __wine_process_init() in loader.c + * + * LOCKING + * First locks objects.mutex, then releases and locks handles.mutex + */ +NTSTATUS ntdll_object_db_init(void) +{ + NTSTATUS ret = 0; + + TRACE_(ntdll_obj)("\n"); + + /* init objects list if not already inited */ + pthread_mutex_lock(&objects.mutex); + if (!objects.list.next) + list_init(&objects.list); + pthread_mutex_unlock(&objects.mutex); + + /* init red-black handle-to-object tree if not already inited */ + pthread_mutex_lock(&handles.mutex); + if (!handles.tree.stack.entries) + { + /* passing handles.tree.functions instead of obj_handles_rb_ops to aid -findirect-inline + * (it might not matter) */ + if (wine_rb_init(&handles.tree, handles.tree.functions) == -1) + { + ERR("Failed to initialize ntdll object handle rbtree.\n"); + ret = ERROR_OUTOFMEMORY; + } + else + atexit(ntdll_objects_cleanup); + } + pthread_mutex_unlock(&handles.mutex); + + return ret; +}
/* @@ -390,6 +955,9 @@ NTSTATUS close_handle( HANDLE handle ) } SERVER_END_REQ; if (fd != -1) close( fd ); + + ntdll_handle_remove( handle ); + return ret; }
+void ntdll_object_release(struct ntdll_object *obj) +{
- int refcount = interlocked_xchg_add(&obj->refcount, -1);
- assert(refcount > 0);
- if (!refcount)
- {
You have an assert followed by a check for a condition that would've failed the assert.
I'm guessing you meant to check for refcount == 1?
When a semaphore is opened or created, the client process now also opens that semaphore and can perform operations on it --- dlls/ntdll/sync.c | 240 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 240 insertions(+)
diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c index 6892732..f4ac276 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -48,6 +48,19 @@ #include <stdlib.h> #include <time.h>
+#ifdef ENABLE_HYBRID_SYNC +# ifdef HAVE_SEMAPHORE_H +# include <semaphore.h> +# endif +# include <fcntl.h> +# include <sys/stat.h> +# include <semaphore.h> +# include <errno.h> +# ifdef HAVE_SYS_TYPES_H +# include <sys/types.h> +# endif +#endif + #include "ntstatus.h" #define WIN32_NO_STATUS #define NONAMELESSUNION @@ -58,9 +71,234 @@ #include "ntdll_misc.h"
WINE_DEFAULT_DEBUG_CHANNEL(ntdll); +WINE_DECLARE_DEBUG_CHANNEL(ntdllsync);
HANDLE keyed_event = NULL;
+static NTSTATUS semaphore_add(const HANDLE h, LONG MaximumCount, unsigned int key, unsigned long debug_server_ptr); + +#ifndef ENABLE_HYBRID_SYNC +static NTSTATUS semaphore_add(const HANDLE h, LONG MaximumCount, unsigned int key, unsigned long debug_server_ptr) +{ + return STATUS_SUCCESS; +} + +static inline NTSTATUS semaphore_up(const HANDLE h, ULONG count, PULONG previous) +{ + return STATUS_NOT_IMPLEMENTED; +} +#else /* ENABLE_HYBRID_SYNC */ + +struct ntdll_semaphore { + struct ntdll_object obj; + sem_t *p; + LONG max; + unsigned int key; + unsigned long server_ptr; +}; + +static void semaphore_dump(const struct ntdll_object *obj, char **start, const char *const end); + +static int semaphore_get_value(struct ntdll_semaphore *sem) +{ + int ret; + if (sem_getvalue(sem->p, &ret) == -1) + { + perror("sem_getvalue"); + /* XXX: is there already a thread safe mechanism for errno descriptions? */ + ERR_(ntdllsync)("sem_getvalue failed with %08x (%s).\n", errno, strerror(errno)); + assert(0); + + return -1; + } + + return ret; +} + +static NTSTATUS semaphore_up(struct ntdll_semaphore *sem, ULONG count, PULONG previous) +{ + int current_value; + + TRACE_(ntdllsync)("(sem = %s, count = %u, previous = %p)\n", + ntdll_object_dump(&sem->obj), count, previous); + + if ((current_value = semaphore_get_value(sem)) == -1) + return STATUS_INVALID_HANDLE; /* ?? */ + + assert(current_value >= 0); + + if (previous) + *previous = (ULONG)current_value; + + if (current_value + count > sem->max) + return STATUS_SEMAPHORE_LIMIT_EXCEEDED; + + while (count--) + { + /* BUG by multiple threads releasing at the same time, it is possible to exceed sem->max here */ + if (sem_post(sem->p) == -1) + { + perror("sem_post"); + assert(0); + exit(1); + /* ?? */ + } + } + TRACE_(ntdllsync)("success\n"); + + ntdll_server_notify_lock_release(); + return STATUS_SUCCESS; +} + +/* client-side waiting not yet implemented */ +static NTSTATUS semaphore_wait(struct ntdll_object *obj, const LARGE_INTEGER *timeout) +{ + TRACE_(ntdllsync)("(obj = %s, timeout = %p)\n", ntdll_object_dump(obj), timeout); + + return STATUS_NOT_IMPLEMENTED; +} + +/* */ +static BOOL semaphore_signaled(struct ntdll_object *obj) +{ + TRACE_(ntdllsync)("(obj = %s)\n", ntdll_object_dump(obj)); + + return semaphore_get_value((void *)obj) > 0; +} + +static NTSTATUS semaphore_trywait(struct ntdll_object *obj) +{ + struct ntdll_semaphore *sem = (void *)obj; + + TRACE_(ntdllsync)("(obj = %s)\n", ntdll_object_dump(obj)); + + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + assert(sem->p); + + if (sem_trywait(sem->p)) + { + switch (errno) { + case EAGAIN: + return STATUS_WAS_LOCKED; + case EDEADLK: + return STATUS_POSSIBLE_DEADLOCK; + default: + perror("sem_trywait"); + exit(1); + } + } + return STATUS_SUCCESS; +} + +static NTSTATUS semaphore_trywait_undo(struct ntdll_object *obj) +{ + struct ntdll_semaphore *sem = (void *)obj; + + TRACE_(ntdllsync)("(obj = %s)\n", ntdll_object_dump(obj)); + + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + assert(sem->p); + + if (sem_post(sem->p) == -1) + { + perror("sem_post"); + exit(1); + } + return STATUS_SUCCESS; +} + +static void semaphore_close(struct ntdll_object *obj) +{ + struct ntdll_semaphore *sem = (void *)obj; + + TRACE_(ntdllsync)("(obj = %s)\n", ntdll_object_dump(obj)); + + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + assert(sem->p); + + if (sem->p) + sem_close(sem->p); +} + +static void semaphore_dump(const struct ntdll_object *obj, char **start, const char *const end) +{ + struct ntdll_semaphore *sem = (void *)obj; + int count; + int sem_value; + + assert(sem); + assert(sem->p); + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + + count = snprintf(*start, end - *start, "%p {obj = ", obj); + if (count < 0) + { + perror("snprintf"); + return; + } + *start += count; + + ntdll_object_dump_base(obj, start, end); + + if (sem_getvalue(sem->p, &sem_value) == -1) + sem_value = -1; + + count = snprintf(*start, end - *start, + ", p = %p {value = %d}, max = %d, key = %08x, server_ptr = %p", + sem->p, sem_value, sem->max, sem->key, (void*)sem->server_ptr); + if (count < 0) + perror("snprintf"); +} + +static struct ntdll_object_ops semaphore_ops = { + semaphore_wait, /* wait */ + semaphore_signaled, /* signaled */ + semaphore_trywait, /* trywait */ + semaphore_trywait_undo, /* trywait_undo */ + semaphore_close, /* close */ + semaphore_dump /* dump */ +}; + +static NTSTATUS semaphore_add(const HANDLE h, LONG MaximumCount, unsigned int key, unsigned long server_ptr) +{ + struct ntdll_semaphore *sem; + sem_t *sem_obj; + char name[32]; + + snprintf(name, sizeof(name) - 1, "/wine-sem-%08x", key); + TRACE_(ntdllsync)("(h = %p, MaximumCount = %d, key = %08x, name = "%s")\n", h, MaximumCount, key, name); + sem_obj = sem_open(name, 0); + + if (sem_obj == SEM_FAILED) + { + perror("sem_open"); + assert(0); + exit(1); + } + + sem = (void *)ntdll_object_new(h, sizeof(struct ntdll_semaphore), NTDLL_OBJ_TYPE_SEMAPHORE, &semaphore_ops); + + if (!sem) + { + ERR_(ntdllsync)("ntdll_object_new failed\n"); + sem_close(sem_obj); + return STATUS_NO_MEMORY; + } + sem->max = MaximumCount; + sem->p = sem_obj; + sem->key = key; + sem->server_ptr = server_ptr; + TRACE_(ntdllsync)("%s\n", ntdll_object_dump(&sem->obj)); + + ntdll_handle_add(&sem->obj); + ntdll_object_release(&sem->obj); + + return STATUS_SUCCESS; +} + +#endif /* ENABLE_HYBRID_SYNC */ + + static inline int interlocked_dec_if_nonzero( int *dest ) { int val, tmp; @@ -184,6 +422,7 @@ NTSTATUS WINAPI NtCreateSemaphore( OUT PHANDLE SemaphoreHandle, if (len) wine_server_add_data( req, attr->ObjectName->Buffer, len ); ret = wine_server_call( req ); *SemaphoreHandle = wine_server_ptr_handle( reply->handle ); + semaphore_add(*SemaphoreHandle, MaximumCount, reply->key, (unsigned long)reply->server_ptr); } SERVER_END_REQ;
@@ -212,6 +451,7 @@ NTSTATUS WINAPI NtOpenSemaphore( OUT PHANDLE SemaphoreHandle, if (len) wine_server_add_data( req, attr->ObjectName->Buffer, len ); ret = wine_server_call( req ); *SemaphoreHandle = wine_server_ptr_handle( reply->handle ); + semaphore_add(*SemaphoreHandle, reply->max, reply->key, (unsigned long)reply->server_ptr); } SERVER_END_REQ; return ret;
When enabled, ReleaseSemaphore will be done by the client process without a server call. However, notification to the server is required and a rather ugly but temporary SIGUSR1 mechanism is implemented (will be changed soon). --- dlls/ntdll/server.c | 14 ++++++++++++++ dlls/ntdll/sync.c | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/server.c b/dlls/ntdll/server.c index 95111ad..763a90d 100644 --- a/dlls/ntdll/server.c +++ b/dlls/ntdll/server.c @@ -682,6 +682,20 @@ unsigned int server_queue_process_apc( HANDLE process, const apc_call_t *call, a } }
+/* notify server that we've released a lock as it won't actually wait (block) + * on any locks and may have a thread suspended waiting for this lock to be + * released */ +void ntdll_server_notify_lock_release(void) +{ + TRACE_(ntdllsync)("server_pid = %u\n", server_pid); + if (server_pid && server_pid != -1) + { + if (kill(server_pid, SIGUSR1)) + { + perror("kill"); + } + } +}
/*********************************************************************** * wine_server_send_fd (NTDLL.@) diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c index f4ac276..2fa90af 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -494,7 +494,38 @@ NTSTATUS WINAPI NtQuerySemaphore( HANDLE handle, SEMAPHORE_INFORMATION_CLASS cla */ NTSTATUS WINAPI NtReleaseSemaphore( HANDLE handle, ULONG count, PULONG previous ) { - NTSTATUS ret; + NTSTATUS ret = 0; +#if 1 + struct ntdll_semaphore *sem = (void *)ntdll_handle_find(handle); + + /* if we know this handle, then manage it client-side, otherwise, it may be a stale handle + * that another thread already closed */ + if (sem) + { + TRACE_(ntdllsync)("handle = %p, count = %u, previous = %p) sem = %s\n", + handle, count, previous, ntdll_object_dump(&sem->obj)); + + assert(sem->p); + assert(sem->obj.type_id == NTDLL_OBJ_TYPE_SEMAPHORE); + + ret = semaphore_up(sem, count, previous); + ntdll_object_release(&sem->obj); + + switch (ret) + { + case STATUS_SUCCESS: + return 1; + + case STATUS_NOT_IMPLEMENTED: + /* fall through to server call */ + break; + + default: + ERR("semaphore_up return %08x\n", ret); + return 0; + } + } +#endif SERVER_START_REQ( release_semaphore ) { req->handle = wine_server_obj_handle( handle );
Sever will now handle SIGUSR1 as notifcation that a client process has released a lock and will walk all threads to see if any are ready to run. This will be replaced by a proper asynchronous message where the client will tell the server exactly which object was released. --- server/signal.c | 12 ++++++++++++ server/thread.c | 20 ++++++++++++++++++++ server/thread.h | 1 + 3 files changed, 33 insertions(+)
diff --git a/server/signal.c b/server/signal.c index 5e4fe33..f241604 100644 --- a/server/signal.c +++ b/server/signal.c @@ -98,6 +98,7 @@ static struct handler *handler_sigterm; static struct handler *handler_sigint; static struct handler *handler_sigchld; static struct handler *handler_sigio; +static struct handler *handler_sigusr1;
static int watchdog;
@@ -239,6 +240,11 @@ static void do_sigio( int signum, siginfo_t *si, void *x ) } #endif
+static void do_sigusr1( int signum ) +{ + do_signal( handler_sigusr1 ); +} + void start_watchdog(void) { alarm( 3 ); @@ -277,6 +283,7 @@ void init_signals(void) if (!(handler_sigint = create_handler( sigint_callback ))) goto error; if (!(handler_sigchld = create_handler( sigchld_callback ))) goto error; if (!(handler_sigio = create_handler( sigio_callback ))) goto error; + if (!(handler_sigusr1 = create_handler( sigusr1_callback ))) goto error;
sigemptyset( &blocked_sigset ); sigaddset( &blocked_sigset, SIGCHLD ); @@ -289,6 +296,7 @@ void init_signals(void) #ifdef SIG_PTHREAD_CANCEL sigaddset( &blocked_sigset, SIG_PTHREAD_CANCEL ); #endif + sigaddset( &blocked_sigset, SIGUSR1 );
action.sa_mask = blocked_sigset; action.sa_flags = 0; @@ -318,6 +326,10 @@ void init_signals(void) action.sa_flags = SA_SIGINFO; sigaction( SIGIO, &action, NULL ); #endif + action.sa_handler = do_sigusr1; + action.sa_flags = 0; + sigaction( SIGUSR1, &action, NULL ); + return;
error: diff --git a/server/thread.c b/server/thread.c index bb21b93..77ff161 100644 --- a/server/thread.c +++ b/server/thread.c @@ -737,6 +737,26 @@ do_satisfied: return -1; }
+/* a less-than-elegant solution to waking up threads after a thread releases a native + * lock and sends us SIGUSR1 */ +static int quick_n_dirty_check_wait_threads(struct process *p, void *smart_dummy) +{ + struct thread *t; + + LIST_FOR_EACH_ENTRY( t, &p->thread_list, struct thread, proc_entry ) + { + if (t->wait) + wake_thread ( t ); + } + + return 0; +} + +void sigusr1_callback(void) +{ + enum_processes(quick_n_dirty_check_wait_threads, NULL); +} + /* send the wakeup signal to a thread */ static int send_thread_wakeup( struct thread *thread, client_ptr_t cookie, int signaled ) { diff --git a/server/thread.h b/server/thread.h index 2821991..40f3e2f 100644 --- a/server/thread.h +++ b/server/thread.h @@ -127,6 +127,7 @@ extern struct thread_snapshot *thread_snap( int *count ); extern struct token *thread_get_impersonation_token( struct thread *thread ); extern int set_thread_affinity( struct thread *thread, affinity_t affinity ); extern int is_cpu_supported( enum cpu_type cpu ); +extern void sigusr1_callback(void);
/* ptrace functions */
server_select can now trywait on native semaphores in a select_op when wait conditions do not require the thread to block. Otherwise, a standard server call is made. --- dlls/ntdll/server.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+)
diff --git a/dlls/ntdll/server.c b/dlls/ntdll/server.c index 763a90d..313dcbb 100644 --- a/dlls/ntdll/server.c +++ b/dlls/ntdll/server.c @@ -83,6 +83,7 @@ #include "ntdll_misc.h"
WINE_DEFAULT_DEBUG_CHANNEL(server); +WINE_DECLARE_DEBUG_CHANNEL(ntdllsync);
/* Some versions of glibc don't define this */ #ifndef SCM_RIGHTS @@ -594,6 +595,138 @@ unsigned int server_select( const select_op_t *select_op, data_size_t size, UINT
memset( &result, 0, sizeof(result) );
+#ifdef ENABLE_HYBRID_SYNC + TRACE_(ntdllsync)("select_op = %p, size = %u, flags = 0x%x, timeout = %p (%lld)\n", + select_op, size, flags, timeout, (timeout ? timeout->QuadPart : 0)); + + if (select_op && (select_op->op == SELECT_WAIT || select_op->op == SELECT_WAIT_ALL)) + { + struct ntdll_object *objs[MAXIMUM_WAIT_OBJECTS]; + size_t nb_locally_lockable_objs = 0; + size_t nb_handles = (size - offsetof( select_op_t, wait.handles )) + / sizeof(select_op->wait.handles[0]); + ssize_t i; + NTSTATUS result; + NTSTATUS ret = STATUS_UNSUCCESSFUL; + ssize_t wait_object = 0; + + /* count the number of locally lockable objects & cache their pointers */ + for (i = 0; i < nb_handles; ++i) + { + objs[i] = ntdll_handle_find(wine_server_ptr_handle(select_op->wait.handles[i])); + if (objs[i] && objs[i]->ops->trywait) + ++nb_locally_lockable_objs; + } + + TRACE_(ntdllsync)("%zd/%zd objects are locally selectable, need %s.\n", + nb_locally_lockable_objs, nb_handles, + (select_op->op == SELECT_WAIT ? "any" : "all")); + + /* bWaitAll = FALSE */ + if (select_op->op == SELECT_WAIT) + { + if (!nb_locally_lockable_objs) + goto local_done; + + /* waitformultipleobjectsex with bWaitAll = FALSE must go in order */ + for (i = 0; i < nb_handles; ++i) + { + /* If we can't lock this one locally, we must do the server call */ + if (!objs[i] || !objs[i]->ops->trywait) + break; + + result = objs[i]->ops->trywait(objs[i]); + if (result == STATUS_SUCCESS) + { + TRACE_(ntdllsync)("Successful local wait any. obj = %p (h = %p)\n", objs[i], objs[i]->h); + + ret = STATUS_SUCCESS; + wait_object = i; + break; + } + else if (result == STATUS_WAS_LOCKED) + continue; + else + ret = result; + } + } + + else /* select_op->op == SELECT_WAIT_ALL (bWaitAll = TRUE)*/ + { + ULONGLONG locked_objs = 0; + assert(MAXIMUM_WAIT_OBJECTS <= sizeof(locked_objs) * 8); + + if (nb_locally_lockable_objs != nb_handles) + goto local_done; + + result = 0; + + /* try to lock all objects */ + for (i = 0; i < nb_handles; ++i) + { + result = objs[i]->ops->trywait(objs[i]); + + if (result == STATUS_WAS_LOCKED) /* normal trywait failure */ + break; + else if (result != STATUS_POSSIBLE_DEADLOCK) + { + WARN("Possible deadlock in thread 0x%x, waiting on handle %p\n", GetCurrentThreadId(), objs[i]->h); + break; + } + else if (result != STATUS_SUCCESS) + { + ERR("object (handle = %p) trywait returned 0x%x\n", objs[i]->h, result); + break; + } + + /* keep track of the objects we've locked, max 64 */ + locked_objs |= 1LL << i; + TRACE_(ntdllsync)("locked object %zd/%zd\n", i, nb_handles); + } + + /* This is a little redundant, if result == STATUS_SUCCESS, then i should also be == nb_handles */ + if (result == STATUS_SUCCESS && i == nb_handles) + { + TRACE_(ntdllsync)("Successful local wait all. count = %zu\n", nb_handles); + ret = STATUS_SUCCESS; + } + /* if not successful then roll back locks */ + else + { + TRACE_(ntdllsync)("rolling back...\n"); + for (; i >= 0; --i) + { + TRACE_(ntdllsync)("rolling back %zd/%zd\n", i, nb_handles); + if (locked_objs & (1 << i)) + objs[i]->ops->trywait_undo(objs[i]); + } + + if (result != STATUS_WAS_LOCKED) + return result; + } + } + +local_done: + + /* release any objects we've grabbed */ + for (i = 0; i < nb_handles; ++i) + if (objs[i]) + ntdll_object_release(objs[i]); + + switch (ret) + { + case STATUS_SUCCESS: + return WAIT_OBJECT_0 + wait_object; + + default: + /* fall through to server call */ + break; + } + } +TRACE_(ntdllsync)("Doing server call.\n"); + +#endif /* ENABLE_HYBRID_SYNC */ + for (;;) { SERVER_START_REQ( select )
The test on line 244 can fail if scheduling happens to preempt the main thread at just the right time, especially if a page fault is hit and I/O is needed or the semaphore implementation is faster than it was when this test was originally written. --- dlls/ntdll/tests/threadpool.c | 62 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 51 insertions(+), 11 deletions(-)
diff --git a/dlls/ntdll/tests/threadpool.c b/dlls/ntdll/tests/threadpool.c index 7be3f0d..5397419 100644 --- a/dlls/ntdll/tests/threadpool.c +++ b/dlls/ntdll/tests/threadpool.c @@ -18,6 +18,9 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */
+#include <stdio.h> +#include <sys/time.h> + #include "ntdll_test.h"
static HMODULE hntdll = 0; @@ -238,7 +241,7 @@ static void test_tp_simple(void) ok(!status, "TpSimpleTryPost failed with status %x\n", status); } pTpReleaseCleanupGroupMembers(group, TRUE, NULL); - ok(userdata < 100, "expected userdata < 100, got %u\n", userdata); + ok(userdata <= 100, "expected userdata <= 100, got %u\n", userdata);
/* cleanup */ pTpReleaseCleanupGroup(group); @@ -1235,11 +1238,15 @@ static void CALLBACK multi_wait_cb(TP_CALLBACK_INSTANCE *instance, void *userdat ReleaseSemaphore(multi_wait_info.semaphore, 1, NULL); }
+#define TP_TEST_SIZE 512 static void test_tp_multi_wait(void) { TP_CALLBACK_ENVIRON environment; - HANDLE semaphores[512]; - TP_WAIT *waits[512]; + HANDLE semaphores[TP_TEST_SIZE]; + TP_WAIT *waits[TP_TEST_SIZE]; + int rand_order[TP_TEST_SIZE]; + struct timeval now; + unsigned int seed; LARGE_INTEGER when; HANDLE semaphore; NTSTATUS status; @@ -1247,7 +1254,7 @@ static void test_tp_multi_wait(void) DWORD result; int i;
- semaphore = CreateSemaphoreW(NULL, 0, 512, NULL); + semaphore = CreateSemaphoreW(NULL, 0, TP_TEST_SIZE, NULL); ok(semaphore != NULL, "failed to create semaphore\n"); multi_wait_info.semaphore = semaphore;
@@ -1261,8 +1268,27 @@ static void test_tp_multi_wait(void) environment.Version = 1; environment.Pool = pool;
+ /* init a randomized index */ + if (gettimeofday (&now, NULL)) + perror("gettimeofday"); + seed = now.tv_sec ^ now.tv_usec; + + memset(&rand_order, 0, TP_TEST_SIZE * sizeof(rand_order[0])); + for (i = 1; i < TP_TEST_SIZE; i++) + { + int j; + for (j = rand_r(&seed) % TP_TEST_SIZE; ; j = (j + 1) % TP_TEST_SIZE) + { + if (!rand_order[j]) + { + rand_order[j] = i; + break; + } + } + } + /* create semaphores and corresponding wait objects */ - for (i = 0; i < sizeof(semaphores)/sizeof(semaphores[0]); i++) + for (i = 0; i < TP_TEST_SIZE; i++) { semaphores[i] = CreateSemaphoreW(NULL, 0, 1, NULL); ok(semaphores[i] != NULL, "failed to create semaphore %i\n", i); @@ -1276,7 +1302,7 @@ static void test_tp_multi_wait(void) }
/* release all semaphores and wait for callback */ - for (i = 0; i < sizeof(semaphores)/sizeof(semaphores[0]); i++) + for (i = 0; i < TP_TEST_SIZE; i++) { multi_wait_info.result = 0; ReleaseSemaphore(semaphores[i], 1, NULL); @@ -1289,7 +1315,7 @@ static void test_tp_multi_wait(void) }
/* repeat the same test in reverse order */ - for (i = sizeof(semaphores)/sizeof(semaphores[0]) - 1; i >= 0; i--) + for (i = TP_TEST_SIZE - 1; i >= 0; i--) { multi_wait_info.result = 0; ReleaseSemaphore(semaphores[i], 1, NULL); @@ -1301,15 +1327,29 @@ static void test_tp_multi_wait(void) pTpSetWait(waits[i], semaphores[i], NULL); }
+ /* repeat again in random order */ + for (i = 0; i < TP_TEST_SIZE; i++) + { + int j = rand_order[i]; + multi_wait_info.result = 0; + ReleaseSemaphore(semaphores[j], 1, NULL); + + result = WaitForSingleObject(semaphore, 100); + ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result); + ok(multi_wait_info.result == j, "expected result %d, got %u\n", j, multi_wait_info.result); + + pTpSetWait(waits[j], semaphores[j], NULL); + } + /* test timeout of wait objects */ multi_wait_info.result = 0; - for (i = 0; i < sizeof(semaphores)/sizeof(semaphores[0]); i++) + for (i = 0; i < TP_TEST_SIZE; i++) { when.QuadPart = (ULONGLONG)50 * -10000; pTpSetWait(waits[i], semaphores[i], &when); }
- for (i = 0; i < sizeof(semaphores)/sizeof(semaphores[0]); i++) + for (i = 0; i < TP_TEST_SIZE; i++) { result = WaitForSingleObject(semaphore, 150); ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result); @@ -1318,14 +1358,14 @@ static void test_tp_multi_wait(void) ok(multi_wait_info.result >> 16, "expected multi_wait_info.result >> 16 != 0\n");
/* destroy the wait objects and semaphores while waiting */ - for (i = 0; i < sizeof(semaphores)/sizeof(semaphores[0]); i++) + for (i = 0; i < TP_TEST_SIZE; i++) { pTpSetWait(waits[i], semaphores[i], NULL); }
Sleep(50);
- for (i = 0; i < sizeof(semaphores)/sizeof(semaphores[0]); i++) + for (i = 0; i < TP_TEST_SIZE; i++) { pTpReleaseWait(waits[i]); NtClose(semaphores[i]);
This revision corrects a few errors, incorrect return value in ReleaseSemaphore and a mistake in the wait when bWaitAll = TRUE (we still need a good test for that). I'm also adding some documentation to the ntdll-internal object management api and one more test to threadpool.
This will be the last revision of the posix (gnu nptl) -based implementation because it does *NOT* work on 32-bit programs when WoW 64/32 is built due to this little bug in glibc (https://sourceware.org/bugzilla/show_bug.cgi?id=11651), but I wanted everything to be correct. It is expected to be bug-free for cases where the wine server and target program are the same ABI.