ntdll: Do not call group cancel callback for finished simple callbacks. (v2)

[Sebastian Lackner]

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

Replaces patch 6/7 of my previous series ( http://source.winehq.org/patches/data/125800 ).
The tests are completely unchanged (and will fail to apply on the testbot).

Changes in v2:
 * Keep the special handling for simple callbacks in tp_object_initialize,
   at least for now. There is still a small risk for race-conditions, and
   I would like to add some more tests before doing such a change.

 dlls/ntdll/tests/threadpool.c |   20 ++++++++++++++++++++
 dlls/ntdll/threadpool.c       |   33 +++++++++++++++++++--------------
 2 files changed, 39 insertions(+), 14 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..04790aa 100644
--- a/dlls/ntdll/threadpool.c
+++ b/dlls/ntdll/threadpool.c
@@ -1885,11 +1885,7 @@ static void tp_object_initialize( struct threadpool_object *object, struct threa
     }
 
     if (is_simple_callback)
-    {
-        tp_object_prepare_shutdown( object );
-        object->shutdown = TRUE;
         tp_object_release( object );
-    }
 }
 
 /***********************************************************************
@@ -2185,6 +2181,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 +2601,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 );
-- 
2.9.0