From: Sebastian Lackner Subject: [6/7] ntdll: Do not call group cancel callback for finished simple callbacks. Message-Id: <31691cec-cd14-d5ea-5111-bcf792ef801e@fds-team.de> Date: Sat, 20 Aug 2016 21:24:31 +0200 Signed-off-by: Sebastian Lackner --- As a nice side effect this also simplifies handling for simple callbacks in tp_object_initialize. :) dlls/ntdll/tests/threadpool.c | 20 +++++++++++++++++ dlls/ntdll/threadpool.c | 48 ++++++++++++++++++------------------------ 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/dlls/ntdll/tests/threadpool.c b/dlls/ntdll/tests/threadpool.c index 7eccb23..af0b667 100644 --- a/dlls/ntdll/tests/threadpool.c +++ b/dlls/ntdll/tests/threadpool.c @@ -705,6 +705,14 @@ static void test_tp_work_scheduler(void) pTpReleasePool(pool); } +static void CALLBACK simple_release_cb(TP_CALLBACK_INSTANCE *instance, void *userdata) +{ + HANDLE *semaphores = userdata; + trace("Running simple release callback\n"); + ReleaseSemaphore(semaphores, 1, NULL); + Sleep(200); /* wait until main thread is in TpReleaseCleanupGroupMembers */ +} + static void CALLBACK work_release_cb(TP_CALLBACK_INSTANCE *instance, void *userdata, TP_WORK *work) { HANDLE semaphore = userdata; @@ -1008,6 +1016,18 @@ static void test_tp_group_cancel(void) ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result); pTpReleaseCleanupGroupMembers(group, TRUE, NULL); + /* terminated simple callbacks should not trigger the group cancel callback */ + memset(&environment, 0, sizeof(environment)); + environment.Version = 1; + environment.Pool = pool; + environment.CleanupGroup = group; + environment.CleanupGroupCancelCallback = unexpected_group_cancel_cleanup_cb; + status = pTpSimpleTryPost(simple_release_cb, semaphores[1], &environment); + ok(!status, "TpSimpleTryPost failed with status %x\n", status); + result = WaitForSingleObject(semaphores[1], 1000); + ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result); + pTpReleaseCleanupGroupMembers(group, TRUE, semaphores); + /* test cancellation callback for objects with multiple instances */ work = NULL; memset(&environment, 0, sizeof(environment)); diff --git a/dlls/ntdll/threadpool.c b/dlls/ntdll/threadpool.c index a572869..0b500a7 100644 --- a/dlls/ntdll/threadpool.c +++ b/dlls/ntdll/threadpool.c @@ -327,7 +327,6 @@ static inline struct threadpool_instance *impl_from_TP_CALLBACK_INSTANCE( TP_CAL static void CALLBACK threadpool_worker_proc( void *param ); static void tp_object_submit( struct threadpool_object *object, BOOL signaled ); -static void tp_object_prepare_shutdown( struct threadpool_object *object ); static BOOL tp_object_release( struct threadpool_object *object ); static struct threadpool *default_threadpool = NULL; @@ -1820,8 +1819,6 @@ static BOOL tp_group_release( struct threadpool_group *group ) static void tp_object_initialize( struct threadpool_object *object, struct threadpool *pool, PVOID userdata, TP_CALLBACK_ENVIRON *environment ) { - BOOL is_simple_callback = (object->type == TP_OBJECT_TYPE_SIMPLE); - object->refcount = 1; object->shutdown = FALSE; @@ -1866,13 +1863,6 @@ static void tp_object_initialize( struct threadpool_object *object, struct threa TRACE( "allocated object %p of type %u\n", object, object->type ); - /* For simple callbacks we have to run tp_object_submit before adding this object - * to the cleanup group. As soon as the cleanup group members are released ->shutdown - * will be set, and tp_object_submit would fail with an assertion. */ - - if (is_simple_callback) - tp_object_submit( object, FALSE ); - if (object->group) { struct threadpool_group *group = object->group; @@ -1883,13 +1873,6 @@ static void tp_object_initialize( struct threadpool_object *object, struct threa object->is_group_member = TRUE; RtlLeaveCriticalSection( &group->cs ); } - - if (is_simple_callback) - { - tp_object_prepare_shutdown( object ); - object->shutdown = TRUE; - tp_object_release( object ); - } } /*********************************************************************** @@ -2185,6 +2168,13 @@ static void CALLBACK threadpool_worker_proc( void *param ) RtlEnterCriticalSection( &pool->cs ); pool->num_busy_workers--; + /* Simple callbacks are automatically shutdown after execution. */ + if (object->type == TP_OBJECT_TYPE_SIMPLE) + { + tp_object_prepare_shutdown( object ); + object->shutdown = TRUE; + } + object->num_running_callbacks--; if (!object->num_pending_callbacks && !object->num_running_callbacks) RtlWakeAllConditionVariable( &object->group_finished_event ); @@ -2598,18 +2588,20 @@ VOID WINAPI TpReleaseCleanupGroupMembers( TP_CLEANUP_GROUP *group, BOOL cancel_p { tp_object_wait( object, TRUE ); - /* Execute group cancellation callback if defined, and if this was actually a group cancel. */ - if ((object->type == TP_OBJECT_TYPE_SIMPLE || !object->shutdown) && - cancel_pending && object->group_cancel_callback) + if (!object->shutdown) { - TRACE( "executing group cancel callback %p(%p, %p)\n", - object->group_cancel_callback, object->userdata, userdata ); - object->group_cancel_callback( object->userdata, userdata ); - TRACE( "callback %p returned\n", object->group_cancel_callback ); - } + /* Execute group cancellation callback if defined, and if this was actually a group cancel. */ + if (cancel_pending && object->group_cancel_callback) + { + TRACE( "executing group cancel callback %p(%p, %p)\n", + object->group_cancel_callback, object->userdata, userdata ); + object->group_cancel_callback( object->userdata, userdata ); + TRACE( "callback %p returned\n", object->group_cancel_callback ); + } - if (object->type != TP_OBJECT_TYPE_SIMPLE && !object->shutdown) - tp_object_release( object ); + if (object->type != TP_OBJECT_TYPE_SIMPLE) + tp_object_release( object ); + } object->shutdown = TRUE; tp_object_release( object ); @@ -2888,6 +2880,8 @@ NTSTATUS WINAPI TpSimpleTryPost( PTP_SIMPLE_CALLBACK callback, PVOID userdata, object->u.simple.callback = callback; tp_object_initialize( object, pool, userdata, environment ); + tp_object_submit( object, FALSE ); + tp_object_release( object ); return STATUS_SUCCESS; } -- 2.9.0