[6/7] ntdll: Do not call group cancel callback for finished simple callbacks.

[Sebastian Lackner]

Signed-off-by: Sebastian Lackner <sebastian at fds-team.de>
---

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