Skip to content

Commit b94a304

Browse files
zfiguraaeikum
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 739ee09 commit b94a304

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
@@ -5855,6 +5855,17 @@ struct get_esync_apc_fd_reply
58555855
struct reply_header __header;
58565856
};
58575857

5858+
5859+
struct esync_msgwait_request
5860+
{
5861+
struct request_header __header;
5862+
int in_msgwait;
5863+
};
5864+
struct esync_msgwait_reply
5865+
{
5866+
struct reply_header __header;
5867+
};
5868+
58585869
enum esync_type
58595870
{
58605871
ESYNC_SEMAPHORE = 1,
@@ -6171,6 +6182,7 @@ enum request
61716182
REQ_open_esync,
61726183
REQ_get_esync_fd,
61736184
REQ_get_esync_apc_fd,
6185+
REQ_esync_msgwait,
61746186
REQ_NB_REQUESTS
61756187
};
61766188

@@ -6480,6 +6492,7 @@ union generic_request
64806492
struct open_esync_request open_esync_request;
64816493
struct get_esync_fd_request get_esync_fd_request;
64826494
struct get_esync_apc_fd_request get_esync_apc_fd_request;
6495+
struct esync_msgwait_request esync_msgwait_request;
64836496
};
64846497
union generic_reply
64856498
{
@@ -6787,8 +6800,9 @@ union generic_reply
67876800
struct open_esync_reply open_esync_reply;
67886801
struct get_esync_fd_reply get_esync_fd_reply;
67896802
struct get_esync_apc_fd_reply get_esync_apc_fd_reply;
6803+
struct esync_msgwait_reply esync_msgwait_reply;
67906804
};
67916805

6792-
#define SERVER_PROTOCOL_VERSION 597
6806+
#define SERVER_PROTOCOL_VERSION 598
67936807

67946808
#endif /* __WINE_WINE_SERVER_PROTOCOL_H */

server/protocol.def

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

39843984
/* Retrieve the fd to wait on for user APCs. */
39853985
@REQ(get_esync_apc_fd)
3986-
@REPLY
3986+
@END
3987+
3988+
/* Notify the server that we are doing a message wait (or done with one). */
3989+
@REQ(esync_msgwait)
3990+
int in_msgwait; /* are we in a message wait? */
39873991
@END
39883992

39893993
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
@@ -414,6 +414,7 @@ DECL_HANDLER(create_esync);
414414
DECL_HANDLER(open_esync);
415415
DECL_HANDLER(get_esync_fd);
416416
DECL_HANDLER(get_esync_apc_fd);
417+
DECL_HANDLER(esync_msgwait);
417418

418419
#ifdef WANT_REQUEST_HANDLERS
419420

@@ -722,6 +723,7 @@ static const req_handler req_handlers[REQ_NB_REQUESTS] =
722723
(req_handler)req_open_esync,
723724
(req_handler)req_get_esync_fd,
724725
(req_handler)req_get_esync_apc_fd,
726+
(req_handler)req_esync_msgwait,
725727
};
726728

727729
C_ASSERT( sizeof(affinity_t) == 8 );
@@ -2475,7 +2477,8 @@ C_ASSERT( FIELD_OFFSET(struct get_esync_fd_reply, type) == 8 );
24752477
C_ASSERT( FIELD_OFFSET(struct get_esync_fd_reply, shm_idx) == 12 );
24762478
C_ASSERT( sizeof(struct get_esync_fd_reply) == 16 );
24772479
C_ASSERT( sizeof(struct get_esync_apc_fd_request) == 16 );
2478-
C_ASSERT( sizeof(struct get_esync_apc_fd_reply) == 8 );
2480+
C_ASSERT( FIELD_OFFSET(struct esync_msgwait_request, in_msgwait) == 12 );
2481+
C_ASSERT( sizeof(struct esync_msgwait_request) == 16 );
24792482

24802483
#endif /* WANT_REQUEST_HANDLERS */
24812484

server/trace.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4647,6 +4647,11 @@ static void dump_get_esync_apc_fd_request( const struct get_esync_apc_fd_request
46474647
{
46484648
}
46494649

4650+
static void dump_esync_msgwait_request( const struct esync_msgwait_request *req )
4651+
{
4652+
fprintf( stderr, " in_msgwait=%d", req->in_msgwait );
4653+
}
4654+
46504655
static const dump_func req_dumpers[REQ_NB_REQUESTS] = {
46514656
(dump_func)dump_new_process_request,
46524657
(dump_func)dump_exec_process_request,
@@ -4950,6 +4955,7 @@ static const dump_func req_dumpers[REQ_NB_REQUESTS] = {
49504955
(dump_func)dump_open_esync_request,
49514956
(dump_func)dump_get_esync_fd_request,
49524957
(dump_func)dump_get_esync_apc_fd_request,
4958+
(dump_func)dump_esync_msgwait_request,
49534959
};
49544960

49554961
static const dump_func reply_dumpers[REQ_NB_REQUESTS] = {
@@ -5255,6 +5261,7 @@ static const dump_func reply_dumpers[REQ_NB_REQUESTS] = {
52555261
(dump_func)dump_open_esync_reply,
52565262
(dump_func)dump_get_esync_fd_reply,
52575263
NULL,
5264+
NULL,
52585265
};
52595266

52605267
static const char * const req_names[REQ_NB_REQUESTS] = {
@@ -5560,6 +5567,7 @@ static const char * const req_names[REQ_NB_REQUESTS] = {
55605567
"open_esync",
55615568
"get_esync_fd",
55625569
"get_esync_apc_fd",
5570+
"esync_msgwait",
55635571
};
55645572

55655573
static const struct

0 commit comments

Comments
 (0)