The problematic call sequence is:
Thread A:
GetQueuedCompletionPortStatus(port); -> blocks
Thread B:
PostQueuedCompletionPortStatus(port);
CloseHandle(port);
Thread A:
unblocks -> INVALID_HANDLE_VALUE
Tests on Windows show that the queued status is always returned to
thread A in this scenario. In Wine, it would sometimes fail due to the
CloseHandle being called before the queued status unblocked the thread,
causing sporadic failures in Mount & Blade: Bannerlord.
This patch works around this by returning a new handle in the
would-block case, which allows the blocked thread to read the last
queued status before destroying the object.
Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com>
---
dlls/ntdll/unix/sync.c | 32 ++++++++++++++++++++++++++------
server/completion.c | 7 +++++++
server/protocol.def | 2 ++
3 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c
index d5d92145503..7fad8250e43 100644
--- a/dlls/ntdll/unix/sync.c
+++ b/dlls/ntdll/unix/sync.c
@@ -1815,6 +1815,7 @@ NTSTATUS WINAPI NtRemoveIoCompletion( HANDLE handle, ULONG_PTR *key, ULONG_PTR *
IO_STATUS_BLOCK *io, LARGE_INTEGER *timeout )
{
NTSTATUS status;
+ HANDLE wait_handle = handle;
TRACE( "(%p, %p, %p, %p, %p)\n", handle, key, value, io, timeout );
@@ -1822,7 +1823,8 @@ NTSTATUS WINAPI NtRemoveIoCompletion( HANDLE handle, ULONG_PTR *key, ULONG_PTR *
{
SERVER_START_REQ( remove_completion )
{
- req->handle = wine_server_obj_handle( handle );
+ req->handle = wine_server_obj_handle( wait_handle );
+ req->alloc_wait_handle = wait_handle == handle;
if (!(status = wine_server_call( req )))
{
*key = reply->ckey;
@@ -1830,12 +1832,22 @@ NTSTATUS WINAPI NtRemoveIoCompletion( HANDLE handle, ULONG_PTR *key, ULONG_PTR *
io->Information = reply->information;
io->u.Status = reply->status;
}
+ else if (status == STATUS_PENDING)
+ {
+ wait_handle = wine_server_ptr_handle( reply->wait_handle );
+ }
}
SERVER_END_REQ;
- if (status != STATUS_PENDING) return status;
- status = NtWaitForSingleObject( handle, FALSE, timeout );
- if (status != WAIT_OBJECT_0) return status;
+ if (status != STATUS_PENDING) goto exit;
+ status = NtWaitForSingleObject( wait_handle, FALSE, timeout );
+ if (status != WAIT_OBJECT_0) goto exit;
}
+
+exit:
+ if (wait_handle != handle)
+ NtClose( wait_handle );
+
+ return status;
}
@@ -1847,6 +1859,7 @@ NTSTATUS WINAPI NtRemoveIoCompletionEx( HANDLE handle, FILE_IO_COMPLETION_INFORM
{
NTSTATUS status;
ULONG i = 0;
+ HANDLE wait_handle = handle;
TRACE( "%p %p %u %p %p %u\n", handle, info, count, written, timeout, alertable );
@@ -1856,7 +1869,8 @@ NTSTATUS WINAPI NtRemoveIoCompletionEx( HANDLE handle, FILE_IO_COMPLETION_INFORM
{
SERVER_START_REQ( remove_completion )
{
- req->handle = wine_server_obj_handle( handle );
+ req->handle = wine_server_obj_handle( wait_handle );
+ req->alloc_wait_handle = wait_handle == handle;
if (!(status = wine_server_call( req )))
{
info[i].CompletionKey = reply->ckey;
@@ -1864,6 +1878,10 @@ NTSTATUS WINAPI NtRemoveIoCompletionEx( HANDLE handle, FILE_IO_COMPLETION_INFORM
info[i].IoStatusBlock.Information = reply->information;
info[i].IoStatusBlock.u.Status = reply->status;
}
+ else if (status == STATUS_PENDING)
+ {
+ wait_handle = wine_server_ptr_handle( reply->wait_handle );
+ }
}
SERVER_END_REQ;
if (status != STATUS_SUCCESS) break;
@@ -1874,10 +1892,12 @@ NTSTATUS WINAPI NtRemoveIoCompletionEx( HANDLE handle, FILE_IO_COMPLETION_INFORM
if (status == STATUS_PENDING) status = STATUS_SUCCESS;
break;
}
- status = NtWaitForSingleObject( handle, alertable, timeout );
+ status = NtWaitForSingleObject( wait_handle, alertable, timeout );
if (status != WAIT_OBJECT_0) break;
}
*written = i ? i : 1;
+ if (wait_handle != handle)
+ NtClose( wait_handle );
return status;
}
diff --git a/server/completion.c b/server/completion.c
index 6933195e72d..6e8a224e3fa 100644
--- a/server/completion.c
+++ b/server/completion.c
@@ -220,7 +220,14 @@ DECL_HANDLER(remove_completion)
entry = list_head( &completion->queue );
if (!entry)
+ {
+ if (req->alloc_wait_handle)
+ reply->wait_handle = duplicate_handle( current->process,
+ req->handle, current->process, 0, 0, DUPLICATE_SAME_ACCESS );
+ else
+ reply->wait_handle = req->handle;
set_error( STATUS_PENDING );
+ }
else
{
list_remove( entry );
diff --git a/server/protocol.def b/server/protocol.def
index ad6d2bb58d0..051a5dbe848 100644
--- a/server/protocol.def
+++ b/server/protocol.def
@@ -3494,11 +3494,13 @@ struct handle_info
/* get completion from completion port queue */
@REQ(remove_completion)
obj_handle_t handle; /* port handle */
+ unsigned int alloc_wait_handle; /* whether to duplicate handle on PENDING result */
@REPLY
apc_param_t ckey; /* completion key */
apc_param_t cvalue; /* completion value */
apc_param_t information; /* IO_STATUS_BLOCK Information */
unsigned int status; /* completion result */
+ obj_handle_t wait_handle; /* if alloc_wait_handle was non-zero, duplicate handle on PENDING result */
@END
--
2.34.0