Skip to content

Commit 9939b92

Browse files
zfiguraimaami
authored andcommitted
ntdll, server: Revert to old implementation of hung queue detection.
By manually notifying the server every time we enter and exit a message wait. The hung queue logic keeps breaking. In the case of bug wine-mirror#9 it was breaking because we were waiting for more than 5 seconds on our queue and then someone sent us a message with SMTO_ABORTIFHUNG. Just stop fighting against the server and try to coöperate with it instead. It takes two extra server calls, but ideally the GUI thread isn't going to be in the same sort of performance- critical code that this patchset was written for.
1 parent 5c4c4e6 commit 9939b92

File tree

6 files changed

+100
-23
lines changed

6 files changed

+100
-23
lines changed

dlls/ntdll/esync.c

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -812,8 +812,8 @@ static void update_grabbed_object( struct esync *obj )
812812

813813
/* A value of STATUS_NOT_IMPLEMENTED returned from this function means that we
814814
* need to delegate to server_select(). */
815-
NTSTATUS esync_wait_objects( DWORD count, const HANDLE *handles, BOOLEAN wait_any,
816-
BOOLEAN alertable, const LARGE_INTEGER *timeout )
815+
static NTSTATUS __esync_wait_objects( DWORD count, const HANDLE *handles,
816+
BOOLEAN wait_any, BOOLEAN alertable, const LARGE_INTEGER *timeout )
817817
{
818818
static const LARGE_INTEGER zero = {0};
819819

@@ -876,22 +876,11 @@ NTSTATUS esync_wait_objects( DWORD count, const HANDLE *handles, BOOLEAN wait_an
876876

877877
if (objs[count - 1] && objs[count - 1]->type == ESYNC_QUEUE)
878878
{
879-
select_op_t select_op;
880-
881879
/* Last object in the list is a queue, which means someone is using
882880
* MsgWaitForMultipleObjects(). We have to wait not only for the server
883881
* fd (signaled on send_message, etc.) but also the USER driver's fd
884882
* (signaled on e.g. X11 events.) */
885883
msgwait = TRUE;
886-
887-
/* We need to let the server know we are doing a message wait, for two
888-
* reasons. First one is WaitForInputIdle(). Second one is checking for
889-
* hung queues. Do it like this. */
890-
select_op.wait.op = SELECT_WAIT;
891-
select_op.wait.handles[0] = wine_server_obj_handle( handles[count - 1] );
892-
ret = server_select( &select_op, offsetof( select_op_t, wait.handles[1] ), 0, &zero );
893-
if (ret != STATUS_WAIT_0 && ret != STATUS_TIMEOUT)
894-
ERR("Unexpected ret %#x\n", ret);
895884
}
896885

897886
if (has_esync && has_server)
@@ -1263,6 +1252,44 @@ NTSTATUS esync_wait_objects( DWORD count, const HANDLE *handles, BOOLEAN wait_an
12631252
return ret;
12641253
}
12651254

1255+
/* We need to let the server know when we are doing a message wait, and when we
1256+
* are done with one, so that all of the code surrounding hung queues works.
1257+
* We also need this for WaitForInputIdle(). */
1258+
static void server_set_msgwait( int in_msgwait )
1259+
{
1260+
SERVER_START_REQ( esync_msgwait )
1261+
{
1262+
req->in_msgwait = in_msgwait;
1263+
wine_server_call( req );
1264+
}
1265+
SERVER_END_REQ;
1266+
}
1267+
1268+
/* This is a very thin wrapper around the proper implementation above. The
1269+
* purpose is to make sure the server knows when we are doing a message wait.
1270+
* This is separated into a wrapper function since there are at least a dozen
1271+
* exit paths from esync_wait_objects(). */
1272+
NTSTATUS esync_wait_objects( DWORD count, const HANDLE *handles, BOOLEAN wait_any,
1273+
BOOLEAN alertable, const LARGE_INTEGER *timeout )
1274+
{
1275+
BOOL msgwait = FALSE;
1276+
struct esync *obj;
1277+
NTSTATUS ret;
1278+
1279+
if (!get_object( handles[count - 1], &obj ) && obj->type == ESYNC_QUEUE)
1280+
{
1281+
msgwait = TRUE;
1282+
server_set_msgwait( 1 );
1283+
}
1284+
1285+
ret = __esync_wait_objects( count, handles, wait_any, alertable, timeout );
1286+
1287+
if (msgwait)
1288+
server_set_msgwait( 0 );
1289+
1290+
return ret;
1291+
}
1292+
12661293
NTSTATUS esync_signal_and_wait( HANDLE signal, HANDLE wait, BOOLEAN alertable,
12671294
const LARGE_INTEGER *timeout )
12681295
{

include/wine/server_protocol.h

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5847,6 +5847,17 @@ struct get_esync_apc_fd_reply
58475847
struct reply_header __header;
58485848
};
58495849

5850+
5851+
struct esync_msgwait_request
5852+
{
5853+
struct request_header __header;
5854+
int in_msgwait;
5855+
};
5856+
struct esync_msgwait_reply
5857+
{
5858+
struct reply_header __header;
5859+
};
5860+
58505861
enum esync_type
58515862
{
58525863
ESYNC_SEMAPHORE = 1,
@@ -6162,6 +6173,7 @@ enum request
61626173
REQ_open_esync,
61636174
REQ_get_esync_fd,
61646175
REQ_get_esync_apc_fd,
6176+
REQ_esync_msgwait,
61656177
REQ_NB_REQUESTS
61666178
};
61676179

@@ -6470,6 +6482,7 @@ union generic_request
64706482
struct open_esync_request open_esync_request;
64716483
struct get_esync_fd_request get_esync_fd_request;
64726484
struct get_esync_apc_fd_request get_esync_apc_fd_request;
6485+
struct esync_msgwait_request esync_msgwait_request;
64736486
};
64746487
union generic_reply
64756488
{
@@ -6776,8 +6789,9 @@ union generic_reply
67766789
struct open_esync_reply open_esync_reply;
67776790
struct get_esync_fd_reply get_esync_fd_reply;
67786791
struct get_esync_apc_fd_reply get_esync_apc_fd_reply;
6792+
struct esync_msgwait_reply esync_msgwait_reply;
67796793
};
67806794

6781-
#define SERVER_PROTOCOL_VERSION 600
6795+
#define SERVER_PROTOCOL_VERSION 601
67826796

67836797
#endif /* __WINE_WINE_SERVER_PROTOCOL_H */

server/protocol.def

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3981,7 +3981,11 @@ struct handle_info
39813981

39823982
/* Retrieve the fd to wait on for user APCs. */
39833983
@REQ(get_esync_apc_fd)
3984-
@REPLY
3984+
@END
3985+
3986+
/* Notify the server that we are doing a message wait (or done with one). */
3987+
@REQ(esync_msgwait)
3988+
int in_msgwait; /* are we in a message wait? */
39853989
@END
39863990

39873991
enum esync_type

server/queue.c

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ struct msg_queue
142142
struct hook_table *hooks; /* hook table */
143143
timeout_t last_get_msg; /* time of last get message call */
144144
int esync_fd; /* esync file descriptor (signalled on message) */
145+
int esync_in_msgwait; /* our thread is currently waiting on us */
145146
};
146147

147148
struct hotkey
@@ -912,7 +913,21 @@ static void cleanup_results( struct msg_queue *queue )
912913
/* check if the thread owning the queue is hung (not checking for messages) */
913914
static int is_queue_hung( struct msg_queue *queue )
914915
{
915-
return (current_time - queue->last_get_msg > 5 * TICKS_PER_SEC);
916+
struct wait_queue_entry *entry;
917+
918+
if (current_time - queue->last_get_msg <= 5 * TICKS_PER_SEC)
919+
return 0; /* less than 5 seconds since last get message -> not hung */
920+
921+
LIST_FOR_EACH_ENTRY( entry, &queue->obj.wait_queue, struct wait_queue_entry, entry )
922+
{
923+
if (get_wait_queue_thread(entry)->queue == queue)
924+
return 0; /* thread is waiting on queue -> not hung */
925+
}
926+
927+
if (do_esync() && queue->esync_in_msgwait)
928+
return 0; /* thread is waiting on queue in absentia -> not hung */
929+
930+
return 1;
916931
}
917932

918933
static int msg_queue_add_queue( struct object *obj, struct wait_queue_entry *entry )
@@ -928,12 +943,6 @@ static int msg_queue_add_queue( struct object *obj, struct wait_queue_entry *ent
928943
}
929944
if (process->idle_event && !(queue->wake_mask & QS_SMRESULT)) set_event( process->idle_event );
930945

931-
/* On Windows, we are considered hung iff we have not somehow processed
932-
* messages OR done a MsgWait call in the last 5 seconds. Note that in the
933-
* latter case repeatedly waiting for 0 seconds is not hung, but waiting
934-
* forever is hung, so this is correct. */
935-
queue->last_get_msg = current_time;
936-
937946
if (queue->fd && list_empty( &obj->wait_queue )) /* first on the queue */
938947
set_fd_events( queue->fd, POLLIN );
939948
add_queue( obj, entry );
@@ -1579,6 +1588,7 @@ static int send_hook_ll_message( struct desktop *desktop, struct message *hardwa
15791588

15801589
if (!(hook_thread = get_first_global_hook( id ))) return 0;
15811590
if (!(queue = hook_thread->queue)) return 0;
1591+
if (is_queue_hung( queue )) return 0;
15821592

15831593
if (!(msg = mem_alloc( sizeof(*msg) ))) return 0;
15841594

@@ -3143,3 +3153,14 @@ DECL_HANDLER(update_rawinput_devices)
31433153
e = find_rawinput_device( 1, 6 );
31443154
current->process->rawinput_kbd = e ? &e->device : NULL;
31453155
}
3156+
3157+
DECL_HANDLER(esync_msgwait)
3158+
{
3159+
struct msg_queue *queue = get_current_queue();
3160+
3161+
if (!queue) return;
3162+
queue->esync_in_msgwait = req->in_msgwait;
3163+
3164+
if (current->process->idle_event && !(queue->wake_mask & QS_SMRESULT))
3165+
set_event( current->process->idle_event );
3166+
}

server/request.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ DECL_HANDLER(create_esync);
413413
DECL_HANDLER(open_esync);
414414
DECL_HANDLER(get_esync_fd);
415415
DECL_HANDLER(get_esync_apc_fd);
416+
DECL_HANDLER(esync_msgwait);
416417

417418
#ifdef WANT_REQUEST_HANDLERS
418419

@@ -720,6 +721,7 @@ static const req_handler req_handlers[REQ_NB_REQUESTS] =
720721
(req_handler)req_open_esync,
721722
(req_handler)req_get_esync_fd,
722723
(req_handler)req_get_esync_apc_fd,
724+
(req_handler)req_esync_msgwait,
723725
};
724726

725727
C_ASSERT( sizeof(affinity_t) == 8 );
@@ -2469,7 +2471,8 @@ C_ASSERT( FIELD_OFFSET(struct get_esync_fd_reply, type) == 8 );
24692471
C_ASSERT( FIELD_OFFSET(struct get_esync_fd_reply, shm_idx) == 12 );
24702472
C_ASSERT( sizeof(struct get_esync_fd_reply) == 16 );
24712473
C_ASSERT( sizeof(struct get_esync_apc_fd_request) == 16 );
2472-
C_ASSERT( sizeof(struct get_esync_apc_fd_reply) == 8 );
2474+
C_ASSERT( FIELD_OFFSET(struct esync_msgwait_request, in_msgwait) == 12 );
2475+
C_ASSERT( sizeof(struct esync_msgwait_request) == 16 );
24732476

24742477
#endif /* WANT_REQUEST_HANDLERS */
24752478

server/trace.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4643,6 +4643,11 @@ static void dump_get_esync_apc_fd_request( const struct get_esync_apc_fd_request
46434643
{
46444644
}
46454645

4646+
static void dump_esync_msgwait_request( const struct esync_msgwait_request *req )
4647+
{
4648+
fprintf( stderr, " in_msgwait=%d", req->in_msgwait );
4649+
}
4650+
46464651
static const dump_func req_dumpers[REQ_NB_REQUESTS] = {
46474652
(dump_func)dump_new_process_request,
46484653
(dump_func)dump_exec_process_request,
@@ -4945,6 +4950,7 @@ static const dump_func req_dumpers[REQ_NB_REQUESTS] = {
49454950
(dump_func)dump_open_esync_request,
49464951
(dump_func)dump_get_esync_fd_request,
49474952
(dump_func)dump_get_esync_apc_fd_request,
4953+
(dump_func)dump_esync_msgwait_request,
49484954
};
49494955

49504956
static const dump_func reply_dumpers[REQ_NB_REQUESTS] = {
@@ -5249,6 +5255,7 @@ static const dump_func reply_dumpers[REQ_NB_REQUESTS] = {
52495255
(dump_func)dump_open_esync_reply,
52505256
(dump_func)dump_get_esync_fd_reply,
52515257
NULL,
5258+
NULL,
52525259
};
52535260

52545261
static const char * const req_names[REQ_NB_REQUESTS] = {
@@ -5553,6 +5560,7 @@ static const char * const req_names[REQ_NB_REQUESTS] = {
55535560
"open_esync",
55545561
"get_esync_fd",
55555562
"get_esync_apc_fd",
5563+
"esync_msgwait",
55565564
};
55575565

55585566
static const struct

0 commit comments

Comments
 (0)