Zebediah Figura : server: Explicitly return whether a select request was immediately signaled.

Alexandre Julliard julliard at winehq.org
Mon Jul 5 16:24:20 CDT 2021


Module: wine
Branch: master
Commit: bedd92ca068434eea7cb6b445a3fcf21b51acbaf
URL:    https://source.winehq.org/git/wine.git/?a=commit;h=bedd92ca068434eea7cb6b445a3fcf21b51acbaf

Author: Zebediah Figura <zfigura at codeweavers.com>
Date:   Mon Jul  5 11:20:01 2021 -0500

server: Explicitly return whether a select request was immediately signaled.

This fixes a regression introduced by 97afac469fbe012e22acc1f1045c88b1004a241f.

If we make a request on an asynchronous device handle, and the IRP handler
returns STATUS_PENDING, wait_async() will return STATUS_PENDING, as intended.
However, if the async object is signaled before the user has a chance to call
wait_async() [e.g. if get_next_device_request is called quickly enough], select
will return STATUS_PENDING immediately, which causes server_select() to think
the object is not signaled, and wait for a select reply forever.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51277
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51295
Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard at winehq.org>

---

 dlls/ntdll/unix/server.c       |  4 +++-
 include/wine/server_protocol.h |  4 ++--
 server/protocol.def            |  1 +
 server/request.h               |  1 +
 server/thread.c                | 32 ++++++++++++++++----------------
 server/trace.c                 |  1 +
 6 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c
index 567f50d0ad7..1d82e806e17 100644
--- a/dlls/ntdll/unix/server.c
+++ b/dlls/ntdll/unix/server.c
@@ -603,6 +603,7 @@ unsigned int server_select( const select_op_t *select_op, data_size_t size, UINT
     apc_call_t call;
     apc_result_t result;
     sigset_t old_set;
+    int signaled;
 
     memset( &result, 0, sizeof(result) );
 
@@ -628,6 +629,7 @@ unsigned int server_select( const select_op_t *select_op, data_size_t size, UINT
                 }
                 if (context) wine_server_set_reply( req, context, 2 * sizeof(*context) );
                 ret = server_call_unlocked( req );
+                signaled    = reply->signaled;
                 apc_handle  = reply->apc_handle;
                 call        = reply->call;
             }
@@ -646,7 +648,7 @@ unsigned int server_select( const select_op_t *select_op, data_size_t size, UINT
             mutex_unlock( mutex );
             mutex = NULL;
         }
-        if (ret != STATUS_PENDING) break;
+        if (signaled) break;
 
         ret = wait_select_reply( &cookie );
     }
diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h
index 867f99a7627..4611f38c5c4 100644
--- a/include/wine/server_protocol.h
+++ b/include/wine/server_protocol.h
@@ -1313,8 +1313,8 @@ struct select_reply
     struct reply_header __header;
     apc_call_t   call;
     obj_handle_t apc_handle;
+    int          signaled;
     /* VARARG(contexts,contexts); */
-    char __pad_60[4];
 };
 #define SELECT_ALERTABLE     1
 #define SELECT_INTERRUPTIBLE 2
@@ -6252,7 +6252,7 @@ union generic_reply
 
 /* ### protocol_version begin ### */
 
-#define SERVER_PROTOCOL_VERSION 724
+#define SERVER_PROTOCOL_VERSION 725
 
 /* ### protocol_version end ### */
 
diff --git a/server/protocol.def b/server/protocol.def
index 10402ca2db3..b4049eb90e7 100644
--- a/server/protocol.def
+++ b/server/protocol.def
@@ -1160,6 +1160,7 @@ typedef struct
 @REPLY
     apc_call_t   call;         /* APC call arguments */
     obj_handle_t apc_handle;   /* handle to next APC */
+    int          signaled;     /* were the handles immediately signaled? */
     VARARG(contexts,contexts); /* suspend context(s) */
 @END
 #define SELECT_ALERTABLE     1
diff --git a/server/request.h b/server/request.h
index a5490fd215a..18a6a2df3c7 100644
--- a/server/request.h
+++ b/server/request.h
@@ -896,6 +896,7 @@ C_ASSERT( FIELD_OFFSET(struct select_request, prev_apc) == 36 );
 C_ASSERT( sizeof(struct select_request) == 40 );
 C_ASSERT( FIELD_OFFSET(struct select_reply, call) == 8 );
 C_ASSERT( FIELD_OFFSET(struct select_reply, apc_handle) == 56 );
+C_ASSERT( FIELD_OFFSET(struct select_reply, signaled) == 60 );
 C_ASSERT( sizeof(struct select_reply) == 64 );
 C_ASSERT( FIELD_OFFSET(struct create_event_request, access) == 12 );
 C_ASSERT( FIELD_OFFSET(struct create_event_request, manual_reset) == 16 );
diff --git a/server/thread.c b/server/thread.c
index e0c520ee37c..e23412c6033 100644
--- a/server/thread.c
+++ b/server/thread.c
@@ -974,8 +974,8 @@ static int signal_object( obj_handle_t handle )
 }
 
 /* select on a list of handles */
-static void select_on( const select_op_t *select_op, data_size_t op_size, client_ptr_t cookie,
-                            int flags, abstime_t when )
+static int select_on( const select_op_t *select_op, data_size_t op_size, client_ptr_t cookie,
+                      int flags, abstime_t when )
 {
     int ret;
     unsigned int count;
@@ -984,7 +984,7 @@ static void select_on( const select_op_t *select_op, data_size_t op_size, client
     switch (select_op->op)
     {
     case SELECT_NONE:
-        if (!wait_on( select_op, 0, NULL, flags, when )) return;
+        if (!wait_on( select_op, 0, NULL, flags, when )) return 1;
         break;
 
     case SELECT_WAIT:
@@ -993,24 +993,24 @@ static void select_on( const select_op_t *select_op, data_size_t op_size, client
         if (op_size < offsetof( select_op_t, wait.handles ) || count > MAXIMUM_WAIT_OBJECTS)
         {
             set_error( STATUS_INVALID_PARAMETER );
-            return;
+            return 1;
         }
         if (!wait_on_handles( select_op, count, select_op->wait.handles, flags, when ))
-            return;
+            return 1;
         break;
 
     case SELECT_SIGNAL_AND_WAIT:
         if (!wait_on_handles( select_op, 1, &select_op->signal_and_wait.wait, flags, when ))
-            return;
+            return 1;
         if (select_op->signal_and_wait.signal)
         {
             if (!signal_object( select_op->signal_and_wait.signal ))
             {
                 end_wait( current, get_error() );
-                return;
+                return 1;
             }
             /* check if we woke ourselves up */
-            if (!current->wait) return;
+            if (!current->wait) return 1;
         }
         break;
 
@@ -1018,23 +1018,23 @@ static void select_on( const select_op_t *select_op, data_size_t op_size, client
     case SELECT_KEYED_EVENT_RELEASE:
         object = (struct object *)get_keyed_event_obj( current->process, select_op->keyed_event.handle,
                          select_op->op == SELECT_KEYED_EVENT_WAIT ? KEYEDEVENT_WAIT : KEYEDEVENT_WAKE );
-        if (!object) return;
+        if (!object) return 1;
         ret = wait_on( select_op, 1, &object, flags, when );
         release_object( object );
-        if (!ret) return;
+        if (!ret) return 1;
         current->wait->key = select_op->keyed_event.key;
         break;
 
     default:
         set_error( STATUS_INVALID_PARAMETER );
-        return;
+        return 1;
     }
 
     if ((ret = check_wait( current )) != -1)
     {
         /* condition is already satisfied */
         set_error( end_wait( current, ret ));
-        return;
+        return 1;
     }
 
     /* now we need to wait */
@@ -1044,12 +1044,12 @@ static void select_on( const select_op_t *select_op, data_size_t op_size, client
                                                       thread_timeout, current->wait )))
         {
             end_wait( current, get_error() );
-            return;
+            return 1;
         }
     }
     current->wait->cookie = cookie;
     set_error( STATUS_PENDING );
-    return;
+    return 0;
 }
 
 /* attempt to wake threads sleeping on the object wait queue */
@@ -1664,7 +1664,7 @@ DECL_HANDLER(select)
         release_object( apc );
     }
 
-    select_on( &select_op, op_size, req->cookie, req->flags, req->timeout );
+    reply->signaled = select_on( &select_op, op_size, req->cookie, req->flags, req->timeout );
 
     if (get_error() == STATUS_USER_APC)
     {
@@ -1684,7 +1684,7 @@ DECL_HANDLER(select)
         }
         release_object( apc );
     }
-    else if (get_error() != STATUS_PENDING && get_reply_max_size() >= sizeof(context_t) &&
+    else if (reply->signaled && get_reply_max_size() >= sizeof(context_t) &&
              current->context && current->suspend_cookie == req->cookie)
     {
         ctx = current->context;
diff --git a/server/trace.c b/server/trace.c
index c65694ad644..447710e8f80 100644
--- a/server/trace.c
+++ b/server/trace.c
@@ -1803,6 +1803,7 @@ static void dump_select_reply( const struct select_reply *req )
 {
     dump_apc_call( " call=", &req->call );
     fprintf( stderr, ", apc_handle=%04x", req->apc_handle );
+    fprintf( stderr, ", signaled=%d", req->signaled );
     dump_varargs_contexts( ", contexts=", cur_size );
 }
 




More information about the wine-cvs mailing list