[PATCH] ntdll and wineserver: Make QueueUserAPC more conformant.
David Koolhoven
david at koolhoven-home.net
Thu Jan 14 10:13:30 CST 2021
Windows developers expect calls made on a terminating thread
to result in ERROR_GEN_FAILURE like a disconnected device,
not ERROR_ACCESS_DENIED.
Change to code added here:
<https://source.winehq.org/git/wine.git/commitdiff/442f5f56d1c3367c9d166ef340a682a390bd57cb>
QueueUserAPC is expected to create a thread specific FIFO
queue. Adding to the end of an unspecified queue on a
different thread removes multithreading optimization
attempts.
Change to code added here:
Thread swap on queueAPC call:
<https://source.winehq.org/git/wine.git/commitdiff/6ca1d1b0812496e670957543294b04abfefe8d78>
Removed a set_error call on test to queue_apc to allow for
other error reasons besides THREAD_IS_TERMINATING to make it
out, and added more informative errror codes inside thread
handlers.
This solves problems seen in earlier WebRTC:
<https://webrtc.googlesource.com/src/+/dbfb58b850c14eeadac5ac66689fb855c79abf36>
This may also improve queue and thread based optimizations,
and prevent race conditions. I wasn't able to find a
Wine-Bug open for this.
Signed-off-by: David Koolhoven <david at koolhoven-home.net>
---
dlls/ntdll/error.c | 2 +-
dlls/ntdll/make_errors | 2 +-
server/thread.c | 61 +++++++++++++++++-------------------------
3 files changed, 27 insertions(+), 38 deletions(-)
diff --git a/dlls/ntdll/error.c b/dlls/ntdll/error.c
index fb630fa9864..3b6b2ecdebe 100644
--- a/dlls/ntdll/error.c
+++ b/dlls/ntdll/error.c
@@ -475,7 +475,7 @@ static const DWORD error_map[1579] =
ERROR_INVALID_PARAMETER, /* c0000048 (STATUS_PORT_ALREADY_SET) */
ERROR_INVALID_PARAMETER, /* c0000049 (STATUS_SECTION_NOT_IMAGE) */
ERROR_SIGNAL_REFUSED, /* c000004a (STATUS_SUSPEND_COUNT_EXCEEDED) */
- ERROR_ACCESS_DENIED, /* c000004b (STATUS_THREAD_IS_TERMINATING) */
+ ERROR_GEN_FAILURE, /* c000004b (STATUS_THREAD_IS_TERMINATING) */
ERROR_INVALID_PARAMETER, /* c000004c (STATUS_BAD_WORKING_SET_LIMIT) */
ERROR_INVALID_PARAMETER, /* c000004d (STATUS_INCOMPATIBLE_FILE_MAP) */
ERROR_INVALID_PARAMETER, /* c000004e (STATUS_SECTION_PROTECTION) */
diff --git a/dlls/ntdll/make_errors b/dlls/ntdll/make_errors
index bedce57fe7a..b7a2a346e5c 100755
--- a/dlls/ntdll/make_errors
+++ b/dlls/ntdll/make_errors
@@ -307,7 +307,7 @@ my %error_map = qw(
STATUS_PORT_ALREADY_SET ERROR_INVALID_PARAMETER
STATUS_SECTION_NOT_IMAGE ERROR_INVALID_PARAMETER
STATUS_SUSPEND_COUNT_EXCEEDED ERROR_SIGNAL_REFUSED
- STATUS_THREAD_IS_TERMINATING ERROR_ACCESS_DENIED
+ STATUS_THREAD_IS_TERMINATING ERROR_GEN_FAILURE
STATUS_BAD_WORKING_SET_LIMIT ERROR_INVALID_PARAMETER
STATUS_INCOMPATIBLE_FILE_MAP ERROR_INVALID_PARAMETER
STATUS_SECTION_PROTECTION ERROR_INVALID_PARAMETER
diff --git a/server/thread.c b/server/thread.c
index 942a8ff8389..9e747271e61 100644
--- a/server/thread.c
+++ b/server/thread.c
@@ -388,7 +388,8 @@ static void cleanup_thread( struct thread *thread )
if (thread->context)
{
- thread->context->status = STATUS_ACCESS_DENIED;
+ thread->context->status = STATUS_THREAD_IS_TERMINATING;
+ set_error( STATUS_THREAD_IS_TERMINATING );
wake_up( &thread->context->obj, 0 );
release_object( thread->context );
thread->context = NULL;
@@ -427,6 +428,7 @@ static void cleanup_thread( struct thread *thread )
static void destroy_thread( struct object *obj )
{
struct thread *thread = (struct thread *)obj;
+ set_error( STATUS_THREAD_IS_TERMINATING ); /* Maybe remove */
assert( obj->ops == &thread_ops );
assert( !thread->debug_ctx ); /* cannot still be debugging something */
@@ -518,7 +520,7 @@ struct thread *get_thread_from_id( thread_id_t id )
struct object *obj = get_ptid_entry( id );
if (obj && obj->ops == &thread_ops) return (struct thread *)grab_object( obj );
- set_error( STATUS_INVALID_CID );
+ set_error( STATUS_NO_LDT );
return NULL;
}
@@ -538,6 +540,7 @@ struct thread *get_thread_from_tid( int tid )
{
if (thread->unix_tid == tid) return thread;
}
+ set_error( STATUS_NO_LDT );
return NULL;
}
@@ -1077,41 +1080,23 @@ static int queue_apc( struct process *process, struct thread *thread, struct thr
{
struct list *queue;
- if (thread && thread->state == TERMINATED && process)
- thread = NULL;
-
- if (!thread) /* find a suitable thread inside the process */
+ if (thread && thread->state == TERMINATED)
{
- struct thread *candidate;
-
- /* first try to find a waiting thread */
- LIST_FOR_EACH_ENTRY( candidate, &process->thread_list, struct thread, proc_entry )
- {
- if (candidate->state == TERMINATED) continue;
- if (is_in_apc_wait( candidate ))
- {
- thread = candidate;
- break;
- }
- }
- if (!thread)
- {
- /* then use the first one that accepts a signal */
- LIST_FOR_EACH_ENTRY( candidate, &process->thread_list, struct thread, proc_entry )
- {
- if (send_thread_signal( candidate, SIGUSR1 ))
- {
- thread = candidate;
- break;
- }
- }
- }
- if (!thread) return 0; /* nothing found */
- queue = get_apc_queue( thread, apc->call.type );
+ set_error( STATUS_THREAD_IS_TERMINATING );
+ return 0;
+ }
+ if (!thread)
+ {
+ set_error( STATUS_NO_LDT );
+ return 0;
}
else
{
- if (thread->state == TERMINATED) return 0;
+ if (thread->state == TERMINATED)
+ {
+ set_error( STATUS_THREAD_IS_TERMINATING );
+ return 0;
+ }
queue = get_apc_queue( thread, apc->call.type );
/* send signal for system APCs if needed */
if (queue == &thread->system_apc && list_empty( queue ) && !is_in_apc_wait( thread ))
@@ -1251,7 +1236,11 @@ int thread_get_inflight_fd( struct thread *thread, int client )
/* kill a thread on the spot */
void kill_thread( struct thread *thread, int violent_death )
{
- if (thread->state == TERMINATED) return; /* already killed */
+ if (thread->state == TERMINATED)
+ {
+ set_error( STATUS_THREAD_IS_TERMINATING );
+ return; /* already killed */
+ }
thread->state = TERMINATED;
thread->exit_time = current_time;
if (current == thread) current = NULL;
@@ -1563,7 +1552,7 @@ DECL_HANDLER(suspend_thread)
if ((thread = get_thread_from_handle( req->handle, THREAD_SUSPEND_RESUME )))
{
- if (thread->state == TERMINATED) set_error( STATUS_ACCESS_DENIED );
+ if (thread->state == TERMINATED) set_error( STATUS_UNSUCCESSFUL );
else reply->count = suspend_thread( thread );
release_object( thread );
}
@@ -1750,7 +1739,7 @@ DECL_HANDLER(queue_apc)
if (thread)
{
- if (!queue_apc( NULL, thread, apc )) set_error( STATUS_THREAD_IS_TERMINATING );
+ queue_apc( NULL, thread, apc );
release_object( thread );
}
else if (process)
--
2.19.1
More information about the wine-devel
mailing list