server: Merge APC processing into the select request.

dlls/kernel32/tests/pipe.c

@@ -1283,9 +1283,6 @@ static DWORD CALLBACK overlapped_server(LPVOID arg)
     SetEvent(a->pipe_created);
 
     ret = WaitForSingleObjectEx(ol.hEvent, INFINITE, 1);
-    if(ret == WAIT_IO_COMPLETION)
-        todo_wine ok(ret == WAIT_OBJECT_0, "ret %x\n", ret);
-    else
     ok(ret == WAIT_OBJECT_0, "ret %x\n", ret);
 
     ret = GetOverlappedResult(pipe, &ol, &num, 1);

dlls/ntdll/sync.c

@@ -658,6 +658,8 @@ static BOOL invoke_apc( const apc_call_t *call, apc_result_t *result )
 
     switch (call->type)
     {
+    case APC_NONE:
+        break;
     case APC_USER:
         call->user.func( call->user.args[0], call->user.args[1], call->user.args[2] );
         user_apc = TRUE;
@@ -787,43 +789,6 @@ static BOOL invoke_apc( const apc_call_t *call, apc_result_t *result )
 }
 
 /***********************************************************************
- *              call_apcs
- *
- * Call outstanding APCs. Return TRUE if a user APC has been run.
- */
-static BOOL call_apcs( BOOL alertable )
-{
-    BOOL user_apc = FALSE;
-    NTSTATUS ret;
-    apc_call_t call;
-    apc_result_t result;
-    HANDLE handle = 0;
-
-    memset( &result, 0, sizeof(result) );
-
-    for (;;)
-    {
-        SERVER_START_REQ( get_apc )
-        {
-            req->alertable = alertable;
-            req->prev      = handle;
-            req->result    = result;
-            if (!(ret = wine_server_call( req )))
-            {
-                handle = reply->handle;
-                call   = reply->call;
-            }
-        }
-        SERVER_END_REQ;
-
-        if (ret) return user_apc;  /* no more APCs */
-
-        user_apc = invoke_apc( &call, &result );
-    }
-}
-
-
-/***********************************************************************
  *           NTDLL_queue_process_apc
  */
 NTSTATUS NTDLL_queue_process_apc( HANDLE process, const apc_call_t *call, apc_result_t *result )
@@ -880,8 +845,14 @@ NTSTATUS NTDLL_wait_for_multiple_objects( UINT count, const HANDLE *handles, UIN
 {
     NTSTATUS ret;
     int cookie;
+    BOOL user_apc = FALSE;
+    obj_handle_t apc_handle = 0;
+    apc_call_t call;
+    apc_result_t result;
     timeout_t abs_timeout = timeout ? timeout->QuadPart : TIMEOUT_INFINITE;
 
+    memset( &result, 0, sizeof(result) );
+
     for (;;)
     {
         SERVER_START_REQ( select )
@@ -889,22 +860,34 @@ NTSTATUS NTDLL_wait_for_multiple_objects( UINT count, const HANDLE *handles, UIN
             req->flags    = flags;
             req->cookie   = &cookie;
             req->signal   = signal_object;
+            req->prev_apc = apc_handle;
             req->timeout  = abs_timeout;
+            wine_server_add_data( req, &result, sizeof(result) );
             wine_server_add_data( req, handles, count * sizeof(HANDLE) );
             ret = wine_server_call( req );
             abs_timeout = reply->timeout;
+            apc_handle  = reply->apc_handle;
+            call        = reply->call;
         }
         SERVER_END_REQ;
         if (ret == STATUS_PENDING) ret = wait_reply( &cookie );
         if (ret != STATUS_USER_APC) break;
-        if (call_apcs( (flags & SELECT_ALERTABLE) != 0 )) break;
+        if (invoke_apc( &call, &result ))
+        {
+            /* if we ran a user apc we have to check once more if an object got signaled,
+             * but we don't want to wait */
+            abs_timeout = 0;
+            user_apc = TRUE;
+        }
         signal_object = 0;  /* don't signal it multiple times */
     }
 
+    if (ret == STATUS_TIMEOUT && user_apc) ret = STATUS_USER_APC;
+
     /* A test on Windows 2000 shows that Windows always yields during
        a wait, but a wait that is hit by an event gets a priority
        boost as well.  This seems to model that behavior the closest.  */
-    if (ret == WAIT_TIMEOUT) NtYieldExecution();
+    if (ret == STATUS_TIMEOUT) NtYieldExecution();
 
     return ret;
 }

include/wine/server_protocol.h

@@ -738,22 +738,6 @@ struct queue_apc_reply
 
 
 
-struct get_apc_request
-{
-    struct request_header __header;
-    int          alertable;
-    obj_handle_t prev;
-    apc_result_t result;
-};
-struct get_apc_reply
-{
-    struct reply_header __header;
-    obj_handle_t handle;
-    apc_call_t   call;
-};
-
-
-
 struct get_apc_result_request
 {
     struct request_header __header;
@@ -853,13 +837,17 @@ struct select_request
     int          flags;
     void*        cookie;
     obj_handle_t signal;
+    obj_handle_t prev_apc;
     timeout_t    timeout;
+    /* VARARG(result,apc_result); */
     /* VARARG(handles,handles); */
 };
 struct select_reply
 {
     struct reply_header __header;
+    obj_handle_t apc_handle;
     timeout_t    timeout;
+    apc_call_t   call;
 };
 #define SELECT_ALL           1
 #define SELECT_ALERTABLE     2
@@ -4086,7 +4074,6 @@ enum request
     REQ_load_dll,
     REQ_unload_dll,
     REQ_queue_apc,
-    REQ_get_apc,
     REQ_get_apc_result,
     REQ_close_handle,
     REQ_set_handle_info,
@@ -4313,7 +4300,6 @@ union generic_request
     struct load_dll_request load_dll_request;
     struct unload_dll_request unload_dll_request;
     struct queue_apc_request queue_apc_request;
-    struct get_apc_request get_apc_request;
     struct get_apc_result_request get_apc_result_request;
     struct close_handle_request close_handle_request;
     struct set_handle_info_request set_handle_info_request;
@@ -4538,7 +4524,6 @@ union generic_reply
     struct load_dll_reply load_dll_reply;
     struct unload_dll_reply unload_dll_reply;
     struct queue_apc_reply queue_apc_reply;
-    struct get_apc_reply get_apc_reply;
     struct get_apc_result_reply get_apc_result_reply;
     struct close_handle_reply close_handle_reply;
     struct set_handle_info_reply set_handle_info_reply;
@@ -4742,6 +4727,6 @@ union generic_reply
     struct make_process_system_reply make_process_system_reply;
 };
 
-#define SERVER_PROTOCOL_VERSION 306
+#define SERVER_PROTOCOL_VERSION 307
 
 #endif /* __WINE_WINE_SERVER_PROTOCOL_H */

server/protocol.def

@@ -658,17 +658,6 @@ typedef union
 @END
 
 
-/* Get next APC to call */
-@REQ(get_apc)
-    int          alertable;    /* is thread alertable? */
-    obj_handle_t prev;         /* handle to previous APC */
-    apc_result_t result;       /* result of previous APC */
-@REPLY
-    obj_handle_t handle;       /* handle to APC */
-    apc_call_t   call;         /* call arguments */
-@END
-
-
 /* Get the result of an APC call */
 @REQ(get_apc_result)
     obj_handle_t handle;       /* handle to the APC */
@@ -736,10 +725,14 @@ typedef union
     int          flags;        /* wait flags (see below) */
     void*        cookie;       /* magic cookie to return to client */
     obj_handle_t signal;       /* object to signal (0 if none) */
+    obj_handle_t prev_apc;     /* handle to previous APC */
     timeout_t    timeout;      /* timeout */
+    VARARG(result,apc_result); /* result of previous APC */
     VARARG(handles,handles);   /* handles to select on */
 @REPLY
+    obj_handle_t apc_handle;   /* handle to next APC */
     timeout_t    timeout;      /* timeout converted to absolute */
+    apc_call_t   call;         /* APC call arguments */
 @END
 #define SELECT_ALL           1
 #define SELECT_ALERTABLE     2

server/request.h

@@ -128,7 +128,6 @@ DECL_HANDLER(resume_thread);
 DECL_HANDLER(load_dll);
 DECL_HANDLER(unload_dll);
 DECL_HANDLER(queue_apc);
-DECL_HANDLER(get_apc);
 DECL_HANDLER(get_apc_result);
 DECL_HANDLER(close_handle);
 DECL_HANDLER(set_handle_info);
@@ -354,7 +353,6 @@ static const req_handler req_handlers[REQ_NB_REQUESTS] =
     (req_handler)req_load_dll,
     (req_handler)req_unload_dll,
     (req_handler)req_queue_apc,
-    (req_handler)req_get_apc,
     (req_handler)req_get_apc_result,
     (req_handler)req_close_handle,
     (req_handler)req_set_handle_info,

server/thread.c

@@ -1149,8 +1149,65 @@ DECL_HANDLER(resume_thread)
 /* select on a handle list */
 DECL_HANDLER(select)
 {
-    unsigned int count = get_req_data_size() / sizeof(obj_handle_t);
-    reply->timeout = select_on( count, req->cookie, get_req_data(), req->flags, req->timeout, req->signal );
+    struct thread_apc *apc;
+    unsigned int count;
+    const apc_result_t *result = get_req_data();
+    const obj_handle_t *handles = (const obj_handle_t *)(result + 1);
+
+    if (get_req_data_size() < sizeof(*result))
+    {
+        set_error( STATUS_INVALID_PARAMETER );
+        return;
+    }
+    count = (get_req_data_size() - sizeof(*result)) / sizeof(obj_handle_t);
+
+    /* first store results of previous apc */
+    if (req->prev_apc)
+    {
+        if (!(apc = (struct thread_apc *)get_handle_obj( current->process, req->prev_apc,
+                                                         0, &thread_apc_ops ))) return;
+        apc->result = *result;
+        apc->executed = 1;
+        if (apc->result.type == APC_CREATE_THREAD)  /* transfer the handle to the caller process */
+        {
+            obj_handle_t handle = duplicate_handle( current->process, apc->result.create_thread.handle,
+                                                    apc->caller->process, 0, 0, DUP_HANDLE_SAME_ACCESS );
+            close_handle( current->process, apc->result.create_thread.handle );
+            apc->result.create_thread.handle = handle;
+            clear_error();  /* ignore errors from the above calls */
+        }
+        else if (apc->result.type == APC_ASYNC_IO)
+        {
+            if (apc->owner) async_set_result( apc->owner, apc->result.async_io.status );
+        }
+        wake_up( &apc->obj, 0 );
+        close_handle( current->process, req->prev_apc );
+        release_object( apc );
+    }
+
+    reply->timeout = select_on( count, req->cookie, handles, req->flags, req->timeout, req->signal );
+
+    if (get_error() == STATUS_USER_APC)
+    {
+        for (;;)
+        {
+            if (!(apc = thread_dequeue_apc( current, !(req->flags & SELECT_ALERTABLE) )))
+                break;
+            /* Optimization: ignore APC_NONE calls, they are only used to
+             * wake up a thread, but since we got here the thread woke up already.
+             */
+            if (apc->call.type != APC_NONE)
+            {
+                if ((reply->apc_handle = alloc_handle( current->process, apc, SYNCHRONIZE, 0 )))
+                    reply->call = apc->call;
+                release_object( apc );
+                break;
+            }
+            apc->executed = 1;
+            wake_up( &apc->obj, 0 );
+            release_object( apc );
+        }
+    }
 }
 
 /* queue an APC for a thread or process */
@@ -1234,59 +1291,6 @@ DECL_HANDLER(queue_apc)
     release_object( apc );
 }
 
-/* get next APC to call */
-DECL_HANDLER(get_apc)
-{
-    struct thread_apc *apc;
-    int system_only = !req->alertable;
-
-    if (req->prev)
-    {
-        if (!(apc = (struct thread_apc *)get_handle_obj( current->process, req->prev,
-                                                         0, &thread_apc_ops ))) return;
-        apc->result = req->result;
-        apc->executed = 1;
-        if (apc->result.type == APC_CREATE_THREAD)  /* transfer the handle to the caller process */
-        {
-            obj_handle_t handle = duplicate_handle( current->process, apc->result.create_thread.handle,
-                                                    apc->caller->process, 0, 0, DUP_HANDLE_SAME_ACCESS );
-            close_handle( current->process, apc->result.create_thread.handle );
-            apc->result.create_thread.handle = handle;
-            clear_error();  /* ignore errors from the above calls */
-        }
-        else if (apc->result.type == APC_ASYNC_IO)
-        {
-            if (apc->owner) async_set_result( apc->owner, apc->result.async_io.status );
-        }
-        wake_up( &apc->obj, 0 );
-        close_handle( current->process, req->prev );
-        release_object( apc );
-    }
-
-    if (current->suspend + current->process->suspend > 0) system_only = 1;
-
-    for (;;)
-    {
-        if (!(apc = thread_dequeue_apc( current, system_only )))
-        {
-            /* no more APCs */
-            set_error( STATUS_PENDING );
-            return;
-        }
-        /* Optimization: ignore APC_NONE calls, they are only used to
-         * wake up a thread, but since we got here the thread woke up already.
-         */
-        if (apc->call.type != APC_NONE) break;
-        apc->executed = 1;
-        wake_up( &apc->obj, 0 );
-        release_object( apc );
-    }
-
-    if ((reply->handle = alloc_handle( current->process, apc, SYNCHRONIZE, 0 )))
-        reply->call = apc->call;
-    release_object( apc );
-}
-
 /* Get the result of an APC call */
 DECL_HANDLER(get_apc_result)
 {

server/trace.c

@@ -301,6 +301,18 @@ static void dump_varargs_ints( data_size_t size )
     remove_data( size );
 }
 
+static void dump_varargs_apc_result( data_size_t size )
+{
+    const apc_result_t *result = cur_data;
+
+    if (size >= sizeof(*result))
+    {
+        dump_apc_result( result );
+        size = sizeof(*result);
+    }
+    remove_data( size );
+}
+
 static void dump_varargs_handles( data_size_t size )
 {
     const obj_handle_t *data = cur_data;
@@ -1014,21 +1026,6 @@ static void dump_queue_apc_reply( const struct queue_apc_reply *req )
     fprintf( stderr, " self=%d", req->self );
 }
 
-static void dump_get_apc_request( const struct get_apc_request *req )
-{
-    fprintf( stderr, " alertable=%d,", req->alertable );
-    fprintf( stderr, " prev=%p,", req->prev );
-    fprintf( stderr, " result=" );
-    dump_apc_result( &req->result );
-}
-
-static void dump_get_apc_reply( const struct get_apc_reply *req )
-{
-    fprintf( stderr, " handle=%p,", req->handle );
-    fprintf( stderr, " call=" );
-    dump_apc_call( &req->call );
-}
-
 static void dump_get_apc_result_request( const struct get_apc_result_request *req )
 {
     fprintf( stderr, " handle=%p", req->handle );
@@ -1103,17 +1100,25 @@ static void dump_select_request( const struct select_request *req )
     fprintf( stderr, " flags=%d,", req->flags );
     fprintf( stderr, " cookie=%p,", req->cookie );
     fprintf( stderr, " signal=%p,", req->signal );
+    fprintf( stderr, " prev_apc=%p,", req->prev_apc );
     fprintf( stderr, " timeout=" );
     dump_timeout( &req->timeout );
     fprintf( stderr, "," );
+    fprintf( stderr, " result=" );
+    dump_varargs_apc_result( cur_size );
+    fputc( ',', stderr );
     fprintf( stderr, " handles=" );
     dump_varargs_handles( cur_size );
 }
 
 static void dump_select_reply( const struct select_reply *req )
 {
+    fprintf( stderr, " apc_handle=%p,", req->apc_handle );
     fprintf( stderr, " timeout=" );
     dump_timeout( &req->timeout );
+    fprintf( stderr, "," );
+    fprintf( stderr, " call=" );
+    dump_apc_call( &req->call );
 }
 
 static void dump_create_event_request( const struct create_event_request *req )
@@ -3558,7 +3563,6 @@ static const dump_func req_dumpers[REQ_NB_REQUESTS] = {
     (dump_func)dump_load_dll_request,
     (dump_func)dump_unload_dll_request,
     (dump_func)dump_queue_apc_request,
-    (dump_func)dump_get_apc_request,
     (dump_func)dump_get_apc_result_request,
     (dump_func)dump_close_handle_request,
     (dump_func)dump_set_handle_info_request,
@@ -3781,7 +3785,6 @@ static const dump_func reply_dumpers[REQ_NB_REQUESTS] = {
     (dump_func)0,
     (dump_func)0,
     (dump_func)dump_queue_apc_reply,
-    (dump_func)dump_get_apc_reply,
     (dump_func)dump_get_apc_result_reply,
     (dump_func)0,
     (dump_func)dump_set_handle_info_reply,
@@ -4004,7 +4007,6 @@ static const char * const req_names[REQ_NB_REQUESTS] = {
     "load_dll",
     "unload_dll",
     "queue_apc",
-    "get_apc",
     "get_apc_result",
     "close_handle",
     "set_handle_info",