Signed-off-by: Paul Gofman pgofman@codeweavers.com --- server/completion.c | 118 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 94 insertions(+), 24 deletions(-)
diff --git a/server/completion.c b/server/completion.c index 6933195e72d..8dd8d4b1247 100644 --- a/server/completion.c +++ b/server/completion.c @@ -56,15 +56,50 @@ struct type_descr completion_type = }, };
-struct completion +struct completion_wait { struct object obj; struct list queue; unsigned int depth; };
+struct completion +{ + struct object obj; + struct completion_wait *wait; +}; + +static void completion_wait_dump( struct object*, int ); +static int completion_wait_signaled( struct object *obj, struct wait_queue_entry *entry ); +static void completion_wait_destroy( struct object * ); + +static const struct object_ops completion_wait_ops = +{ + sizeof(struct completion_wait), /* size */ + &no_type, /* type */ + completion_wait_dump, /* dump */ + add_queue, /* add_queue */ + remove_queue, /* remove_queue */ + completion_wait_signaled, /* signaled */ + no_satisfied, /* satisfied */ + no_signal, /* signal */ + no_get_fd, /* get_fd */ + default_map_access, /* map_access */ + default_get_sd, /* get_sd */ + default_set_sd, /* set_sd */ + no_get_full_name, /* get_full_name */ + no_lookup_name, /* lookup_name */ + no_link_name, /* link_name */ + NULL, /* unlink_name */ + no_open_file, /* open_file */ + no_kernel_obj_list, /* get_kernel_obj_list */ + no_close_handle, /* close_handle */ + completion_wait_destroy /* destroy */ +}; + static void completion_dump( struct object*, int ); -static int completion_signaled( struct object *obj, struct wait_queue_entry *entry ); +static int completion_add_queue( struct object *obj, struct wait_queue_entry *entry ); +static void completion_remove_queue( struct object *obj, struct wait_queue_entry *entry ); static void completion_destroy( struct object * );
static const struct object_ops completion_ops = @@ -72,9 +107,9 @@ static const struct object_ops completion_ops = sizeof(struct completion), /* size */ &completion_type, /* type */ completion_dump, /* dump */ - add_queue, /* add_queue */ - remove_queue, /* remove_queue */ - completion_signaled, /* signaled */ + completion_add_queue, /* add_queue */ + completion_remove_queue, /* remove_queue */ + NULL, /* signaled */ no_satisfied, /* satisfied */ no_signal, /* signal */ no_get_fd, /* get_fd */ @@ -100,30 +135,63 @@ struct comp_msg unsigned int status; };
-static void completion_destroy( struct object *obj) +static void completion_wait_destroy( struct object *obj) { - struct completion *completion = (struct completion *) obj; + struct completion_wait *wait = (struct completion_wait *)obj; struct comp_msg *tmp, *next;
- LIST_FOR_EACH_ENTRY_SAFE( tmp, next, &completion->queue, struct comp_msg, queue_entry ) + LIST_FOR_EACH_ENTRY_SAFE( tmp, next, &wait->queue, struct comp_msg, queue_entry ) { free( tmp ); } }
+static void completion_wait_dump( struct object *obj, int verbose ) +{ + struct completion_wait *wait = (struct completion_wait *)obj; + + assert( obj->ops == &completion_wait_ops ); + fprintf( stderr, "Completion depth=%u\n", wait->depth ); +} + +static int completion_wait_signaled( struct object *obj, struct wait_queue_entry *entry ) +{ + struct completion_wait *wait = (struct completion_wait *)obj; + + assert( obj->ops == &completion_wait_ops ); + return !list_empty( &wait->queue ); +} + static void completion_dump( struct object *obj, int verbose ) { - struct completion *completion = (struct completion *) obj; + struct completion *completion = (struct completion *)obj; + + assert( obj->ops == &completion_ops ); + completion->wait->obj.ops->dump( &completion->wait->obj, verbose ); +} + +static int completion_add_queue( struct object *obj, struct wait_queue_entry *entry ) +{ + struct completion *completion = (struct completion *)obj; + + assert( obj->ops == &completion_ops ); + return completion->wait->obj.ops->add_queue( &completion->wait->obj, entry ); +} + +static void completion_remove_queue( struct object *obj, struct wait_queue_entry *entry ) +{ + struct completion *completion = (struct completion *)obj;
assert( obj->ops == &completion_ops ); - fprintf( stderr, "Completion depth=%u\n", completion->depth ); + completion->wait->obj.ops->remove_queue( &completion->wait->obj, entry ); }
-static int completion_signaled( struct object *obj, struct wait_queue_entry *entry ) +static void completion_destroy( struct object *obj ) { struct completion *completion = (struct completion *)obj;
- return !list_empty( &completion->queue ); + assert( obj->ops == &completion_ops ); + release_object( &completion->wait->obj ); }
static struct completion *create_completion( struct object *root, const struct unicode_str *name, @@ -132,15 +200,17 @@ static struct completion *create_completion( struct object *root, const struct u { struct completion *completion;
- if ((completion = create_named_object( root, &completion_ops, name, attr, sd ))) + if (!(completion = create_named_object( root, &completion_ops, name, attr, sd ))) return NULL; + if (get_error() == STATUS_OBJECT_NAME_EXISTS) return completion; + if (!(completion->wait = alloc_object( &completion_wait_ops ))) { - if (get_error() != STATUS_OBJECT_NAME_EXISTS) - { - list_init( &completion->queue ); - completion->depth = 0; - } + release_object( completion ); + set_error( STATUS_NO_MEMORY ); + return NULL; }
+ list_init( &completion->wait->queue ); + completion->wait->depth = 0; return completion; }
@@ -162,9 +232,9 @@ void add_completion( struct completion *completion, apc_param_t ckey, apc_param_ msg->status = status; msg->information = information;
- list_add_tail( &completion->queue, &msg->queue_entry ); - completion->depth++; - wake_up( &completion->obj, 1 ); + list_add_tail( &completion->wait->queue, &msg->queue_entry ); + completion->wait->depth++; + wake_up( &completion->wait->obj, 1 ); }
/* create a completion */ @@ -218,13 +288,13 @@ DECL_HANDLER(remove_completion)
if (!completion) return;
- entry = list_head( &completion->queue ); + entry = list_head( &completion->wait->queue ); if (!entry) set_error( STATUS_PENDING ); else { list_remove( entry ); - completion->depth--; + completion->wait->depth--; msg = LIST_ENTRY( entry, struct comp_msg, queue_entry ); reply->ckey = msg->ckey; reply->cvalue = msg->cvalue; @@ -243,7 +313,7 @@ DECL_HANDLER(query_completion)
if (!completion) return;
- reply->depth = completion->depth; + reply->depth = completion->wait->depth;
release_object( completion ); }
Based on patches by Alexey Prokhin.
Signed-off-by: Paul Gofman pgofman@codeweavers.com --- dlls/ntdll/tests/sync.c | 65 +++++++++++++++++++++++++++++++++++++++++ server/completion.c | 25 ++++++++++++---- 2 files changed, 85 insertions(+), 5 deletions(-)
diff --git a/dlls/ntdll/tests/sync.c b/dlls/ntdll/tests/sync.c index f930767a8b0..562df0f66b0 100644 --- a/dlls/ntdll/tests/sync.c +++ b/dlls/ntdll/tests/sync.c @@ -837,6 +837,70 @@ static void test_tid_alert( char **argv ) CloseHandle( pi.hThread ); }
+static HANDLE test_close_io_completion_port_ready, test_close_io_completion_test_ready; +static HANDLE test_close_io_completion_port; + +static DWORD WINAPI test_close_io_completion_thread(void *param) +{ + FILE_IO_COMPLETION_INFORMATION info; + IO_STATUS_BLOCK iosb; + ULONG_PTR key, value; + NTSTATUS status; + ULONG count; + DWORD ret; + + ret = WaitForSingleObject( test_close_io_completion_port_ready, INFINITE ); + ok( ret == WAIT_OBJECT_0, "Got unexpected ret %#x.\n", ret ); + SetEvent( test_close_io_completion_test_ready ); + status = NtRemoveIoCompletion( test_close_io_completion_port, &key, &value, &iosb, NULL ); + if (status == STATUS_INVALID_HANDLE) + skip( "Handle closed before wait started.\n" ); + else + ok( status == STATUS_ABANDONED_WAIT_0, "Got unexpected status %#x.\n", status ); + + ret = WaitForSingleObject( test_close_io_completion_port_ready, INFINITE ); + ok( ret == WAIT_OBJECT_0, "Got unexpected ret %#x.\n", ret ); + SetEvent( test_close_io_completion_test_ready ); + count = 0xdeadbeef; + status = NtRemoveIoCompletionEx( test_close_io_completion_port, &info, 1, &count, NULL, FALSE ); + ok( count == 1, "Got unexpected count %u.\n", count ); + if (status == STATUS_INVALID_HANDLE) + skip( "Handle closed before wait started.\n" ); + else + ok( status == STATUS_ABANDONED_WAIT_0, "Got unexpected status %#x.\n", status ); + + return 0; +} + +static void test_close_io_completion(void) +{ + NTSTATUS status; + unsigned int i; + HANDLE thread; + DWORD ret; + + test_close_io_completion_port_ready = CreateEventA(NULL, FALSE, FALSE, NULL); + test_close_io_completion_test_ready = CreateEventA(NULL, FALSE, FALSE, NULL); + + thread = CreateThread( NULL, 0, test_close_io_completion_thread, NULL, 0, NULL ); + ok( !!thread, "Failed to create thread, error %u.\n", GetLastError() ); + + for (i = 0; i < 2; ++i) + { + status = NtCreateIoCompletion( &test_close_io_completion_port, IO_COMPLETION_ALL_ACCESS, NULL, 0 ); + ok( !status, "Got unexpected status %#x.\n", status ); + ret = SignalObjectAndWait( test_close_io_completion_port_ready, test_close_io_completion_test_ready, + INFINITE, FALSE ); + ok( ret == WAIT_OBJECT_0, "Got unexpected ret %#x.\n", ret ); + Sleep(10); + status = pNtClose( test_close_io_completion_port ); + ok( !status, "Got unexpected status %#x.\n", status ); + } + + WaitForSingleObject( thread, INFINITE ); + CloseHandle( thread ); +} + START_TEST(sync) { HMODULE module = GetModuleHandleA("ntdll.dll"); @@ -884,4 +948,5 @@ START_TEST(sync) test_keyed_events(); test_resource(); test_tid_alert( argv ); + test_close_io_completion(); } diff --git a/server/completion.c b/server/completion.c index 8dd8d4b1247..27a7f3e3e50 100644 --- a/server/completion.c +++ b/server/completion.c @@ -56,11 +56,14 @@ struct type_descr completion_type = }, };
+struct completion; + struct completion_wait { - struct object obj; - struct list queue; - unsigned int depth; + struct object obj; + struct completion *completion; + struct list queue; + unsigned int depth; };
struct completion @@ -71,6 +74,7 @@ struct completion
static void completion_wait_dump( struct object*, int ); static int completion_wait_signaled( struct object *obj, struct wait_queue_entry *entry ); +static void completion_wait_satisfied( struct object *obj, struct wait_queue_entry *entry ); static void completion_wait_destroy( struct object * );
static const struct object_ops completion_wait_ops = @@ -81,7 +85,7 @@ static const struct object_ops completion_wait_ops = add_queue, /* add_queue */ remove_queue, /* remove_queue */ completion_wait_signaled, /* signaled */ - no_satisfied, /* satisfied */ + completion_wait_satisfied, /* satisfied */ no_signal, /* signal */ no_get_fd, /* get_fd */ default_map_access, /* map_access */ @@ -159,7 +163,15 @@ static int completion_wait_signaled( struct object *obj, struct wait_queue_entry struct completion_wait *wait = (struct completion_wait *)obj;
assert( obj->ops == &completion_wait_ops ); - return !list_empty( &wait->queue ); + return !wait->completion || !list_empty( &wait->queue ); +} + +static void completion_wait_satisfied( struct object *obj, struct wait_queue_entry *entry ) +{ + struct completion_wait *wait = (struct completion_wait *)obj; + + assert( obj->ops == &completion_wait_ops ); + if (!wait->completion) make_wait_abandoned( entry ); }
static void completion_dump( struct object *obj, int verbose ) @@ -191,6 +203,8 @@ static void completion_destroy( struct object *obj ) struct completion *completion = (struct completion *)obj;
assert( obj->ops == &completion_ops ); + completion->wait->completion = NULL; + wake_up( &completion->wait->obj, 0 ); release_object( &completion->wait->obj ); }
@@ -209,6 +223,7 @@ static struct completion *create_completion( struct object *root, const struct u return NULL; }
+ completion->wait->completion = completion; list_init( &completion->wait->queue ); completion->wait->depth = 0; return completion;
Based on a patch by Alexey Prokhin.
Signed-off-by: Paul Gofman pgofman@codeweavers.com --- dlls/kernelbase/sync.c | 2 ++ dlls/ntdll/tests/sync.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/dlls/kernelbase/sync.c b/dlls/kernelbase/sync.c index ab963c62221..a5522b1d72a 100644 --- a/dlls/kernelbase/sync.c +++ b/dlls/kernelbase/sync.c @@ -1067,6 +1067,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetQueuedCompletionStatus( HANDLE port, LPDWORD co }
if (status == STATUS_TIMEOUT) SetLastError( WAIT_TIMEOUT ); + else if (status == STATUS_ABANDONED_WAIT_0) SetLastError( ERROR_ABANDONED_WAIT_0 ); else SetLastError( RtlNtStatusToDosError(status) ); return FALSE; } @@ -1087,6 +1088,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetQueuedCompletionStatusEx( HANDLE port, OVERLAPP written, get_nt_timeout( &time, timeout ), alertable ); if (ret == STATUS_SUCCESS) return TRUE; else if (ret == STATUS_TIMEOUT) SetLastError( WAIT_TIMEOUT ); + else if (ret == STATUS_ABANDONED_WAIT_0) SetLastError( ERROR_ABANDONED_WAIT_0 ); else if (ret == STATUS_USER_APC) SetLastError( WAIT_IO_COMPLETION ); else SetLastError( RtlNtStatusToDosError(ret) ); return FALSE; diff --git a/dlls/ntdll/tests/sync.c b/dlls/ntdll/tests/sync.c index 562df0f66b0..97e8ba1b778 100644 --- a/dlls/ntdll/tests/sync.c +++ b/dlls/ntdll/tests/sync.c @@ -846,8 +846,9 @@ static DWORD WINAPI test_close_io_completion_thread(void *param) IO_STATUS_BLOCK iosb; ULONG_PTR key, value; NTSTATUS status; + OVERLAPPED *ov; + DWORD ret, err; ULONG count; - DWORD ret;
ret = WaitForSingleObject( test_close_io_completion_port_ready, INFINITE ); ok( ret == WAIT_OBJECT_0, "Got unexpected ret %#x.\n", ret ); @@ -869,6 +870,29 @@ static DWORD WINAPI test_close_io_completion_thread(void *param) else ok( status == STATUS_ABANDONED_WAIT_0, "Got unexpected status %#x.\n", status );
+ ret = WaitForSingleObject( test_close_io_completion_port_ready, INFINITE ); + ok( ret == WAIT_OBJECT_0, "Got unexpected ret %#x.\n", ret ); + SetEvent( test_close_io_completion_test_ready ); + ov = (void *)0xdeadbeaf; + SetLastError(0xdeadbeef); + ret = GetQueuedCompletionStatus( test_close_io_completion_port, &count, &key, &ov, INFINITE ); + err = GetLastError(); + ok( !ret && (err == ERROR_ABANDONED_WAIT_0 || err == ERROR_INVALID_HANDLE), + "Got unexpected ret %#x, error %u.\n", ret, err ); + ok( !ov, "Got unexpected ov %p.\n", ov ); + + ret = WaitForSingleObject( test_close_io_completion_port_ready, INFINITE ); + ok( ret == WAIT_OBJECT_0, "Got unexpected ret %#x.\n", ret ); + SetEvent( test_close_io_completion_test_ready ); + ov = (void *)0xdeadbeaf; + SetLastError(0xdeadbeef); + ret = GetQueuedCompletionStatusEx( test_close_io_completion_port, (OVERLAPPED_ENTRY *)&info, 1, + &count, INFINITE, FALSE ); + err = GetLastError(); + ok( !ret && (err == ERROR_ABANDONED_WAIT_0 || err == ERROR_INVALID_HANDLE), + "Got unexpected ret %#x, error %u.\n", ret, err ); + ok( count == 1, "Got unexpected count %u.\n", count ); + return 0; }
@@ -885,7 +909,7 @@ static void test_close_io_completion(void) thread = CreateThread( NULL, 0, test_close_io_completion_thread, NULL, 0, NULL ); ok( !!thread, "Failed to create thread, error %u.\n", GetLastError() );
- for (i = 0; i < 2; ++i) + for (i = 0; i < 4; ++i) { status = NtCreateIoCompletion( &test_close_io_completion_port, IO_COMPLETION_ALL_ACCESS, NULL, 0 ); ok( !status, "Got unexpected status %#x.\n", status );
Based on the problem analysis by Andrew Eikum.
Signed-off-by: Paul Gofman pgofman@codeweavers.com --- Supersedes 220433-220435.
The added 'waited' request parameter is not needed for NtRemoveIoCompletion[Ex] logic but the application may wait on completion port directly.
dlls/ntdll/unix/sync.c | 6 ++++++ server/completion.c | 36 ++++++++++++++++++++++++++++++------ server/protocol.def | 1 + server/thread.c | 2 ++ server/thread.h | 1 + 5 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index d5d92145503..bc6a06726b0 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; + int waited = 0;
TRACE( "(%p, %p, %p, %p, %p)\n", handle, key, value, io, timeout );
@@ -1823,6 +1824,7 @@ NTSTATUS WINAPI NtRemoveIoCompletion( HANDLE handle, ULONG_PTR *key, ULONG_PTR * SERVER_START_REQ( remove_completion ) { req->handle = wine_server_obj_handle( handle ); + req->waited = waited; if (!(status = wine_server_call( req ))) { *key = reply->ckey; @@ -1835,6 +1837,7 @@ NTSTATUS WINAPI NtRemoveIoCompletion( HANDLE handle, ULONG_PTR *key, ULONG_PTR * if (status != STATUS_PENDING) return status; status = NtWaitForSingleObject( handle, FALSE, timeout ); if (status != WAIT_OBJECT_0) return status; + waited = 1; } }
@@ -1846,6 +1849,7 @@ NTSTATUS WINAPI NtRemoveIoCompletionEx( HANDLE handle, FILE_IO_COMPLETION_INFORM ULONG *written, LARGE_INTEGER *timeout, BOOLEAN alertable ) { NTSTATUS status; + int waited = 0; ULONG i = 0;
TRACE( "%p %p %u %p %p %u\n", handle, info, count, written, timeout, alertable ); @@ -1857,6 +1861,7 @@ NTSTATUS WINAPI NtRemoveIoCompletionEx( HANDLE handle, FILE_IO_COMPLETION_INFORM SERVER_START_REQ( remove_completion ) { req->handle = wine_server_obj_handle( handle ); + req->waited = waited; if (!(status = wine_server_call( req ))) { info[i].CompletionKey = reply->ckey; @@ -1876,6 +1881,7 @@ NTSTATUS WINAPI NtRemoveIoCompletionEx( HANDLE handle, FILE_IO_COMPLETION_INFORM } status = NtWaitForSingleObject( handle, alertable, timeout ); if (status != WAIT_OBJECT_0) break; + waited = 1; } *written = i ? i : 1; return status; diff --git a/server/completion.c b/server/completion.c index 27a7f3e3e50..12948277d79 100644 --- a/server/completion.c +++ b/server/completion.c @@ -169,9 +169,16 @@ static int completion_wait_signaled( struct object *obj, struct wait_queue_entry static void completion_wait_satisfied( struct object *obj, struct wait_queue_entry *entry ) { struct completion_wait *wait = (struct completion_wait *)obj; + struct thread *thread;
assert( obj->ops == &completion_wait_ops ); - if (!wait->completion) make_wait_abandoned( entry ); + if (wait->completion) + { + thread = get_wait_queue_thread( entry ); + if (thread->locked_completion) release_object( thread->locked_completion ); + thread->locked_completion = grab_object( obj ); + } + else make_wait_abandoned( entry ); }
static void completion_dump( struct object *obj, int verbose ) @@ -297,19 +304,36 @@ DECL_HANDLER(add_completion) /* get completion from completion port */ DECL_HANDLER(remove_completion) { - struct completion* completion = get_completion_obj( current->process, req->handle, IO_COMPLETION_MODIFY_STATE ); + struct completion* completion; + struct completion_wait *wait; struct list *entry; struct comp_msg *msg;
- if (!completion) return; + if (req->waited && (wait = (struct completion_wait *)current->locked_completion)) + current->locked_completion = NULL; + else + { + if (current->locked_completion) + { + release_object( current->locked_completion ); + current->locked_completion = NULL; + } + completion = get_completion_obj( current->process, req->handle, IO_COMPLETION_MODIFY_STATE ); + if (!completion) return; + + wait = (struct completion_wait *)grab_object( completion->wait ); + release_object( completion ); + }
- entry = list_head( &completion->wait->queue ); + assert( wait->obj.ops == &completion_wait_ops ); + + entry = list_head( &wait->queue ); if (!entry) set_error( STATUS_PENDING ); else { list_remove( entry ); - completion->wait->depth--; + wait->depth--; msg = LIST_ENTRY( entry, struct comp_msg, queue_entry ); reply->ckey = msg->ckey; reply->cvalue = msg->cvalue; @@ -318,7 +342,7 @@ DECL_HANDLER(remove_completion) free( msg ); }
- release_object( completion ); + release_object( wait ); }
/* get queue depth for completion port */ diff --git a/server/protocol.def b/server/protocol.def index efe0d22cbc4..d2c42d595f7 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3501,6 +3501,7 @@ struct handle_info /* get completion from completion port queue */ @REQ(remove_completion) obj_handle_t handle; /* port handle */ + int waited; /* port was just successfully waited on */ @REPLY apc_param_t ckey; /* completion key */ apc_param_t cvalue; /* completion value */ diff --git a/server/thread.c b/server/thread.c index e9240a05659..146bf0e1bae 100644 --- a/server/thread.c +++ b/server/thread.c @@ -250,6 +250,7 @@ static inline void init_thread_structure( struct thread *thread )
thread->creation_time = current_time; thread->exit_time = 0; + thread->locked_completion = NULL;
list_init( &thread->mutex_list ); list_init( &thread->system_apc ); @@ -452,6 +453,7 @@ static void destroy_thread( struct object *obj ) release_object( thread->process ); if (thread->id) free_ptid( thread->id ); if (thread->token) release_object( thread->token ); + if (thread->locked_completion) release_object( thread->locked_completion ); }
/* dump a thread on stdout for debugging purposes */ diff --git a/server/thread.h b/server/thread.h index 8dcf966a90a..e9442b59d9a 100644 --- a/server/thread.h +++ b/server/thread.h @@ -90,6 +90,7 @@ struct thread struct list kernel_object; /* list of kernel object pointers */ data_size_t desc_len; /* thread description length in bytes */ WCHAR *desc; /* thread description string */ + struct object *locked_completion; /* completion port wait object successfully waited by the thread */ };
extern struct thread *current;