Skip to content

Commit

Permalink
ntdll, server: Revert to old implementation of hung queue detection.
Browse files Browse the repository at this point in the history
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 #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.
  • Loading branch information
zfigura authored and Guy1524 committed Jan 7, 2020
1 parent a70819c commit a0b78ce
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 21 deletions.
53 changes: 40 additions & 13 deletions dlls/ntdll/esync.c
Original file line number Diff line number Diff line change
Expand Up @@ -831,8 +831,8 @@ static void update_grabbed_object( struct esync *obj )

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

Expand Down Expand Up @@ -895,22 +895,11 @@ NTSTATUS esync_wait_objects( DWORD count, const HANDLE *handles, BOOLEAN wait_an

if (objs[count - 1] && objs[count - 1]->type == ESYNC_QUEUE)
{
select_op_t select_op;

/* Last object in the list is a queue, which means someone is using
* MsgWaitForMultipleObjects(). We have to wait not only for the server
* fd (signaled on send_message, etc.) but also the USER driver's fd
* (signaled on e.g. X11 events.) */
msgwait = TRUE;

/* We need to let the server know we are doing a message wait, for two
* reasons. First one is WaitForInputIdle(). Second one is checking for
* hung queues. Do it like this. */
select_op.wait.op = SELECT_WAIT;
select_op.wait.handles[0] = wine_server_obj_handle( handles[count - 1] );
ret = server_select( &select_op, offsetof( select_op_t, wait.handles[1] ), 0, &zero );
if (ret != STATUS_WAIT_0 && ret != STATUS_TIMEOUT)
ERR("Unexpected ret %#x\n", ret);
}

if (has_esync && has_server)
Expand Down Expand Up @@ -1283,6 +1272,44 @@ NTSTATUS esync_wait_objects( DWORD count, const HANDLE *handles, BOOLEAN wait_an
return ret;
}

/* We need to let the server know when we are doing a message wait, and when we
* are done with one, so that all of the code surrounding hung queues works.
* We also need this for WaitForInputIdle(). */
static void server_set_msgwait( int in_msgwait )
{
SERVER_START_REQ( esync_msgwait )
{
req->in_msgwait = in_msgwait;
wine_server_call( req );
}
SERVER_END_REQ;
}

/* This is a very thin wrapper around the proper implementation above. The
* purpose is to make sure the server knows when we are doing a message wait.
* This is separated into a wrapper function since there are at least a dozen
* exit paths from esync_wait_objects(). */
NTSTATUS esync_wait_objects( DWORD count, const HANDLE *handles, BOOLEAN wait_any,
BOOLEAN alertable, const LARGE_INTEGER *timeout )
{
BOOL msgwait = FALSE;
struct esync *obj;
NTSTATUS ret;

if (!get_object( handles[count - 1], &obj ) && obj->type == ESYNC_QUEUE)
{
msgwait = TRUE;
server_set_msgwait( 1 );
}

ret = __esync_wait_objects( count, handles, wait_any, alertable, timeout );

if (msgwait)
server_set_msgwait( 0 );

return ret;
}

NTSTATUS esync_signal_and_wait( HANDLE signal, HANDLE wait, BOOLEAN alertable,
const LARGE_INTEGER *timeout )
{
Expand Down
6 changes: 5 additions & 1 deletion server/protocol.def
Original file line number Diff line number Diff line change
Expand Up @@ -4068,7 +4068,11 @@ struct handle_info

/* Retrieve the fd to wait on for user APCs. */
@REQ(get_esync_apc_fd)
@REPLY
@END

/* Notify the server that we are doing a message wait (or done with one). */
@REQ(esync_msgwait)
int in_msgwait; /* are we in a message wait? */
@END

enum esync_type
Expand Down
35 changes: 28 additions & 7 deletions server/queue.c
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ struct msg_queue
struct thread *thread; /* reference to the thread owning the queue */
struct fd *fd; /* optional file descriptor to poll */
int esync_fd; /* esync file descriptor (signalled on message) */
int esync_in_msgwait; /* our thread is currently waiting on us */
unsigned int wake_bits; /* wakeup bits */
unsigned int wake_mask; /* wakeup mask */
unsigned int changed_bits; /* changed wakeup bits */
Expand Down Expand Up @@ -975,7 +976,21 @@ static void cleanup_results( struct msg_queue *queue )
/* check if the thread owning the queue is hung (not checking for messages) */
static int is_queue_hung( struct msg_queue *queue )
{
return (current_time - queue->last_get_msg > 5 * TICKS_PER_SEC);
struct wait_queue_entry *entry;

if (current_time - queue->last_get_msg <= 5 * TICKS_PER_SEC)
return 0; /* less than 5 seconds since last get message -> not hung */

LIST_FOR_EACH_ENTRY( entry, &queue->obj.wait_queue, struct wait_queue_entry, entry )
{
if (get_wait_queue_thread(entry)->queue == queue)
return 0; /* thread is waiting on queue -> not hung */
}

if (do_esync() && queue->esync_in_msgwait)
return 0; /* thread is waiting on queue in absentia -> not hung */

return 1;
}

static int msg_queue_add_queue( struct object *obj, struct wait_queue_entry *entry )
Expand All @@ -991,12 +1006,6 @@ static int msg_queue_add_queue( struct object *obj, struct wait_queue_entry *ent
}
if (process->idle_event && !(queue->wake_mask & QS_SMRESULT)) set_event( process->idle_event );

/* On Windows, we are considered hung iff we have not somehow processed
* messages OR done a MsgWait call in the last 5 seconds. Note that in the
* latter case repeatedly waiting for 0 seconds is not hung, but waiting
* forever is hung, so this is correct. */
queue->last_get_msg = current_time;

if (queue->fd && list_empty( &obj->wait_queue )) /* first on the queue */
set_fd_events( queue->fd, POLLIN );
add_queue( obj, entry );
Expand Down Expand Up @@ -1688,6 +1697,7 @@ static int send_hook_ll_message( struct desktop *desktop, struct message *hardwa

if (!(hook_thread = get_first_global_hook( id ))) return 0;
if (!(queue = hook_thread->queue)) return 0;
if (is_queue_hung( queue )) return 0;

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

Expand Down Expand Up @@ -3295,3 +3305,14 @@ DECL_HANDLER(update_rawinput_devices)
e = find_rawinput_device( 1, 6 );
current->process->rawinput_kbd = e ? &e->device : NULL;
}

DECL_HANDLER(esync_msgwait)
{
struct msg_queue *queue = get_current_queue();

if (!queue) return;
queue->esync_in_msgwait = req->in_msgwait;

if (current->process->idle_event && !(queue->wake_mask & QS_SMRESULT))
set_event( current->process->idle_event );
}

0 comments on commit a0b78ce

Please sign in to comment.