Skip to content

Commit 416f57c

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 6349127 commit 416f57c

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
@@ -5728,6 +5728,17 @@ struct get_esync_apc_fd_reply
57285728
struct reply_header __header;
57295729
};
57305730

5731+
5732+
struct esync_msgwait_request
5733+
{
5734+
struct request_header __header;
5735+
int in_msgwait;
5736+
};
5737+
struct esync_msgwait_reply
5738+
{
5739+
struct reply_header __header;
5740+
};
5741+
57315742
enum esync_type
57325743
{
57335744
ESYNC_SEMAPHORE = 1,
@@ -6037,6 +6048,7 @@ enum request
60376048
REQ_open_esync,
60386049
REQ_get_esync_fd,
60396050
REQ_get_esync_apc_fd,
6051+
REQ_esync_msgwait,
60406052
REQ_NB_REQUESTS
60416053
};
60426054

@@ -6339,6 +6351,7 @@ union generic_request
63396351
struct open_esync_request open_esync_request;
63406352
struct get_esync_fd_request get_esync_fd_request;
63416353
struct get_esync_apc_fd_request get_esync_apc_fd_request;
6354+
struct esync_msgwait_request esync_msgwait_request;
63426355
};
63436356
union generic_reply
63446357
{
@@ -6639,8 +6652,9 @@ union generic_reply
66396652
struct open_esync_reply open_esync_reply;
66406653
struct get_esync_fd_reply get_esync_fd_reply;
66416654
struct get_esync_apc_fd_reply get_esync_apc_fd_reply;
6655+
struct esync_msgwait_reply esync_msgwait_reply;
66426656
};
66436657

6644-
#define SERVER_PROTOCOL_VERSION 583
6658+
#define SERVER_PROTOCOL_VERSION 584
66456659

66466660
#endif /* __WINE_WINE_SERVER_PROTOCOL_H */

server/protocol.def

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

39013901
/* Retrieve the fd to wait on for user APCs. */
39023902
@REQ(get_esync_apc_fd)
3903-
@REPLY
3903+
@END
3904+
3905+
/* Notify the server that we are doing a message wait (or done with one). */
3906+
@REQ(esync_msgwait)
3907+
int in_msgwait; /* are we in a message wait? */
39043908
@END
39053909

39063910
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
@@ -910,7 +911,21 @@ static void cleanup_results( struct msg_queue *queue )
910911
/* check if the thread owning the queue is hung (not checking for messages) */
911912
static int is_queue_hung( struct msg_queue *queue )
912913
{
913-
return (current_time - queue->last_get_msg > 5 * TICKS_PER_SEC);
914+
struct wait_queue_entry *entry;
915+
916+
if (current_time - queue->last_get_msg <= 5 * TICKS_PER_SEC)
917+
return 0; /* less than 5 seconds since last get message -> not hung */
918+
919+
LIST_FOR_EACH_ENTRY( entry, &queue->obj.wait_queue, struct wait_queue_entry, entry )
920+
{
921+
if (get_wait_queue_thread(entry)->queue == queue)
922+
return 0; /* thread is waiting on queue -> not hung */
923+
}
924+
925+
if (do_esync() && queue->esync_in_msgwait)
926+
return 0; /* thread is waiting on queue in absentia -> not hung */
927+
928+
return 1;
914929
}
915930

916931
static int msg_queue_add_queue( struct object *obj, struct wait_queue_entry *entry )
@@ -926,12 +941,6 @@ static int msg_queue_add_queue( struct object *obj, struct wait_queue_entry *ent
926941
}
927942
if (process->idle_event && !(queue->wake_mask & QS_SMRESULT)) set_event( process->idle_event );
928943

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

15781587
if (!(hook_thread = get_first_global_hook( id ))) return 0;
15791588
if (!(queue = hook_thread->queue)) return 0;
1589+
if (is_queue_hung( queue )) return 0;
15801590

15811591
if (!(msg = mem_alloc( sizeof(*msg) ))) return 0;
15821592

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

server/request.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,7 @@ DECL_HANDLER(create_esync);
407407
DECL_HANDLER(open_esync);
408408
DECL_HANDLER(get_esync_fd);
409409
DECL_HANDLER(get_esync_apc_fd);
410+
DECL_HANDLER(esync_msgwait);
410411

411412
#ifdef WANT_REQUEST_HANDLERS
412413

@@ -708,6 +709,7 @@ static const req_handler req_handlers[REQ_NB_REQUESTS] =
708709
(req_handler)req_open_esync,
709710
(req_handler)req_get_esync_fd,
710711
(req_handler)req_get_esync_apc_fd,
712+
(req_handler)req_esync_msgwait,
711713
};
712714

713715
C_ASSERT( sizeof(affinity_t) == 8 );
@@ -2438,7 +2440,8 @@ C_ASSERT( FIELD_OFFSET(struct get_esync_fd_reply, type) == 8 );
24382440
C_ASSERT( FIELD_OFFSET(struct get_esync_fd_reply, shm_idx) == 12 );
24392441
C_ASSERT( sizeof(struct get_esync_fd_reply) == 16 );
24402442
C_ASSERT( sizeof(struct get_esync_apc_fd_request) == 16 );
2441-
C_ASSERT( sizeof(struct get_esync_apc_fd_reply) == 8 );
2443+
C_ASSERT( FIELD_OFFSET(struct esync_msgwait_request, in_msgwait) == 12 );
2444+
C_ASSERT( sizeof(struct esync_msgwait_request) == 16 );
24422445

24432446
#endif /* WANT_REQUEST_HANDLERS */
24442447

server/trace.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4588,6 +4588,11 @@ static void dump_get_esync_apc_fd_request( const struct get_esync_apc_fd_request
45884588
{
45894589
}
45904590

4591+
static void dump_esync_msgwait_request( const struct esync_msgwait_request *req )
4592+
{
4593+
fprintf( stderr, " in_msgwait=%d", req->in_msgwait );
4594+
}
4595+
45914596
static const dump_func req_dumpers[REQ_NB_REQUESTS] = {
45924597
(dump_func)dump_new_process_request,
45934598
(dump_func)dump_exec_process_request,
@@ -4884,6 +4889,7 @@ static const dump_func req_dumpers[REQ_NB_REQUESTS] = {
48844889
(dump_func)dump_open_esync_request,
48854890
(dump_func)dump_get_esync_fd_request,
48864891
(dump_func)dump_get_esync_apc_fd_request,
4892+
(dump_func)dump_esync_msgwait_request,
48874893
};
48884894

48894895
static const dump_func reply_dumpers[REQ_NB_REQUESTS] = {
@@ -5182,6 +5188,7 @@ static const dump_func reply_dumpers[REQ_NB_REQUESTS] = {
51825188
(dump_func)dump_open_esync_reply,
51835189
(dump_func)dump_get_esync_fd_reply,
51845190
NULL,
5191+
NULL,
51855192
};
51865193

51875194
static const char * const req_names[REQ_NB_REQUESTS] = {
@@ -5480,6 +5487,7 @@ static const char * const req_names[REQ_NB_REQUESTS] = {
54805487
"open_esync",
54815488
"get_esync_fd",
54825489
"get_esync_apc_fd",
5490+
"esync_msgwait",
54835491
};
54845492

54855493
static const struct

0 commit comments

Comments
 (0)