wine/wine-staging
0
0
Fork 0
mirror of https://gitlab.winehq.org/wine/wine-staging.git synced 2025-01-28 22:04:43 -08:00
Code Issues Packages Projects Releases Wiki Activity

Added patches to fix multiple bugs related to threadpool cleanup groups and cancel callbacks.

Browse Source
This commit is contained in:
Sebastian Lackner 2016-08-20 22:51:09 +02:00
parent 17396cec6d
commit f438f86e52
9 changed files with 987 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

134
patches/ntdll-ThreadpoolCleanupGroup/0001-ntdll-tests-Use-longer-waits-to-reduce-risk-of-rando.patch Normal file
View File

@ -0,0 +1,134 @@
From a6213f025a63ad3b2a086c028dae1ff30892af5a Mon Sep 17 00:00:00 2001
From: Sebastian Lackner <sebastian@fds-team.de>
Date: Sat, 20 Aug 2016 21:18:33 +0200
Subject: ntdll/tests: Use longer waits to reduce risk of random failures on
the testbot.
Signed-off-by: Sebastian Lackner <sebastian@fds-team.de>
---
dlls/ntdll/tests/threadpool.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/dlls/ntdll/tests/threadpool.c b/dlls/ntdll/tests/threadpool.c
index 1d8b1f9..9b4b522 100644
--- a/dlls/ntdll/tests/threadpool.c
+++ b/dlls/ntdll/tests/threadpool.c
@@ -579,14 +579,14 @@ static void test_tp_simple(void)
static void CALLBACK work_cb(TP_CALLBACK_INSTANCE *instance, void *userdata, TP_WORK *work)
{
trace("Running work callback\n");
- Sleep(10);
+ Sleep(100);
InterlockedIncrement((LONG *)userdata);
}
static void CALLBACK work2_cb(TP_CALLBACK_INSTANCE *instance, void *userdata, TP_WORK *work)
{
trace("Running work2 callback\n");
- Sleep(10);
+ Sleep(100);
InterlockedExchangeAdd((LONG *)userdata, 0x10000);
}
@@ -599,11 +599,12 @@ static void test_tp_work(void)
LONG userdata;
int i;
- /* allocate new threadpool */
+ /* allocate new threadpool with only one thread */
pool = NULL;
status = pTpAllocPool(&pool, NULL);
ok(!status, "TpAllocPool failed with status %x\n", status);
ok(pool != NULL, "expected pool != NULL\n");
+ pTpSetPoolMaxThreads(pool, 1);
/* allocate new work item */
work = NULL;
@@ -614,12 +615,12 @@ static void test_tp_work(void)
ok(!status, "TpAllocWork failed with status %x\n", status);
ok(work != NULL, "expected work != NULL\n");
- /* post 10 identical work items at once */
+ /* post 5 identical work items at once */
userdata = 0;
- for (i = 0; i < 10; i++)
+ for (i = 0; i < 5; i++)
pTpPostWork(work);
pTpWaitForWork(work, FALSE);
- ok(userdata == 10, "expected userdata = 10, got %u\n", userdata);
+ ok(userdata == 5, "expected userdata = 5, got %u\n", userdata);
/* add more tasks and cancel them immediately */
userdata = 0;
@@ -643,13 +644,11 @@ static void test_tp_work_scheduler(void)
LONG userdata;
int i;
- /* allocate new threadpool */
+ /* allocate new threadpool with only one thread */
pool = NULL;
status = pTpAllocPool(&pool, NULL);
ok(!status, "TpAllocPool failed with status %x\n", status);
ok(pool != NULL, "expected pool != NULL\n");
-
- /* we limit the pool to a single thread */
pTpSetPoolMaxThreads(pool, 1);
/* create a cleanup group */
@@ -683,7 +682,7 @@ static void test_tp_work_scheduler(void)
pTpPostWork(work);
for (i = 0; i < 10; i++)
pTpPostWork(work2);
- Sleep(30);
+ Sleep(500);
pTpWaitForWork(work, TRUE);
pTpWaitForWork(work2, TRUE);
ok(userdata & 0xffff, "expected userdata & 0xffff != 0, got %u\n", userdata & 0xffff);
@@ -691,14 +690,14 @@ static void test_tp_work_scheduler(void)
/* test TpReleaseCleanupGroupMembers on a work item */
userdata = 0;
- for (i = 0; i < 100; i++)
- pTpPostWork(work);
for (i = 0; i < 10; i++)
+ pTpPostWork(work);
+ for (i = 0; i < 3; i++)
pTpPostWork(work2);
pTpReleaseCleanupGroupMembers(group, FALSE, NULL);
pTpWaitForWork(work, TRUE);
- ok((userdata & 0xffff) < 100, "expected userdata & 0xffff < 100, got %u\n", userdata & 0xffff);
- ok((userdata >> 16) == 10, "expected userdata >> 16 == 10, got %u\n", userdata >> 16);
+ ok((userdata & 0xffff) < 10, "expected userdata & 0xffff < 10, got %u\n", userdata & 0xffff);
+ ok((userdata >> 16) == 3, "expected userdata >> 16 == 3, got %u\n", userdata >> 16);
/* cleanup */
pTpReleaseWork(work);
@@ -720,6 +719,7 @@ static void CALLBACK group_cancel_cb(TP_CALLBACK_INSTANCE *instance, void *userd
ok(status == STATUS_TOO_MANY_THREADS || broken(status == 1) /* Win Vista / 2008 */,
"expected STATUS_TOO_MANY_THREADS, got %08x\n", status);
+ ReleaseSemaphore(semaphores[1], 1, NULL);
result = WaitForSingleObject(semaphores[0], 1000);
ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result);
ReleaseSemaphore(semaphores[1], 1, NULL);
@@ -781,6 +781,8 @@ static void test_tp_group_cancel(void)
environment.Pool = pool;
status = pTpSimpleTryPost(group_cancel_cb, semaphores, &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);
memset(&environment, 0, sizeof(environment));
environment.Version = 1;
@@ -816,7 +818,7 @@ static void test_tp_group_cancel(void)
/* check if we get multiple cancellation callbacks */
group_cancel_tid = 0xdeadbeef;
pTpReleaseCleanupGroupMembers(group, TRUE, &userdata2);
- ok(userdata <= 8, "expected userdata <= 8, got %u\n", userdata);
+ ok(userdata <= 5, "expected userdata <= 5, got %u\n", userdata);
ok(userdata2 == 1, "expected only one cancellation callback, got %u\n", userdata2);
ok(group_cancel_tid == GetCurrentThreadId(), "expected tid %x, got %x\n",
GetCurrentThreadId(), group_cancel_tid);
--
2.9.0

142
patches/ntdll-ThreadpoolCleanupGroup/0002-ntdll-Allow-to-release-threadpool-objects-while-wait.patch Normal file
View File

@ -0,0 +1,142 @@
From 696ecf266d29fb3120a1348a47fe55e65da2f0d6 Mon Sep 17 00:00:00 2001
From: Sebastian Lackner <sebastian@fds-team.de>
Date: Sat, 20 Aug 2016 21:22:29 +0200
Subject: ntdll: Allow to release threadpool objects while waiting for group.
Signed-off-by: Sebastian Lackner <sebastian@fds-team.de>
---
dlls/ntdll/threadpool.c | 51 +++++++++++++++++++++++++------------------------
1 file changed, 26 insertions(+), 25 deletions(-)
diff --git a/dlls/ntdll/threadpool.c b/dlls/ntdll/threadpool.c
index 5d5b49d..eea5a88 100644
--- a/dlls/ntdll/threadpool.c
+++ b/dlls/ntdll/threadpool.c
@@ -2,7 +2,7 @@
* Thread pooling
*
* Copyright (c) 2006 Robert Shearman
- * Copyright (c) 2014-2015 Sebastian Lackner
+ * Copyright (c) 2014-2016 Sebastian Lackner
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -327,7 +327,7 @@ 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_shutdown( struct threadpool_object *object );
+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;
@@ -1886,7 +1886,8 @@ static void tp_object_initialize( struct threadpool_object *object, struct threa
if (is_simple_callback)
{
- tp_object_shutdown( object );
+ tp_object_prepare_shutdown( object );
+ object->shutdown = TRUE;
tp_object_release( object );
}
}
@@ -2001,19 +2002,16 @@ static void tp_object_wait( struct threadpool_object *object, BOOL group_wait )
}
/***********************************************************************
- * tp_object_shutdown (internal)
+ * tp_object_prepare_shutdown (internal)
*
- * Marks a threadpool object for shutdown (which means that no further
- * tasks can be submitted).
+ * Prepares a threadpool object for shutdown.
*/
-static void tp_object_shutdown( struct threadpool_object *object )
+static void tp_object_prepare_shutdown( struct threadpool_object *object )
{
if (object->type == TP_OBJECT_TYPE_TIMER)
tp_timerqueue_unlock( object );
else if (object->type == TP_OBJECT_TYPE_WAIT)
tp_waitqueue_unlock( object );
-
- object->shutdown = TRUE;
}
/***********************************************************************
@@ -2574,23 +2572,18 @@ VOID WINAPI TpReleaseCleanupGroupMembers( TP_CLEANUP_GROUP *group, BOOL cancel_p
assert( object->group == this );
assert( object->is_group_member );
- /* Simple callbacks are very special. The user doesn't hold any reference, so
- * they would be released too early. Add one additional temporary reference. */
- if (object->type == TP_OBJECT_TYPE_SIMPLE)
+ if (interlocked_inc( &object->refcount ) == 1)
{
- if (interlocked_inc( &object->refcount ) == 1)
- {
- /* Object is basically already destroyed, but group reference
- * was not deleted yet. We can safely ignore this object. */
- interlocked_dec( &object->refcount );
- list_remove( &object->group_entry );
- object->is_group_member = FALSE;
- continue;
- }
+ /* Object is basically already destroyed, but group reference
+ * was not deleted yet. We can safely ignore this object. */
+ interlocked_dec( &object->refcount );
+ list_remove( &object->group_entry );
+ object->is_group_member = FALSE;
+ continue;
}
object->is_group_member = FALSE;
- tp_object_shutdown( object );
+ tp_object_prepare_shutdown( object );
}
/* Move members to a new temporary list */
@@ -2612,6 +2605,11 @@ VOID WINAPI TpReleaseCleanupGroupMembers( TP_CLEANUP_GROUP *group, BOOL cancel_p
LIST_FOR_EACH_ENTRY_SAFE( object, next, &members, struct threadpool_object, group_entry )
{
tp_object_wait( object, TRUE );
+
+ if (object->type != TP_OBJECT_TYPE_SIMPLE && !object->shutdown)
+ tp_object_release( object );
+
+ object->shutdown = TRUE;
tp_object_release( object );
}
}
@@ -2638,7 +2636,8 @@ VOID WINAPI TpReleaseTimer( TP_TIMER *timer )
TRACE( "%p\n", timer );
- tp_object_shutdown( this );
+ tp_object_prepare_shutdown( this );
+ this->shutdown = TRUE;
tp_object_release( this );
}
@@ -2651,7 +2650,8 @@ VOID WINAPI TpReleaseWait( TP_WAIT *wait )
TRACE( "%p\n", wait );
- tp_object_shutdown( this );
+ tp_object_prepare_shutdown( this );
+ this->shutdown = TRUE;
tp_object_release( this );
}
@@ -2664,7 +2664,8 @@ VOID WINAPI TpReleaseWork( TP_WORK *work )
TRACE( "%p\n", work );
- tp_object_shutdown( this );
+ tp_object_prepare_shutdown( this );
+ this->shutdown = TRUE;
tp_object_release( this );
}
--
2.9.0

148
patches/ntdll-ThreadpoolCleanupGroup/0003-ntdll-tests-Add-tests-for-releasing-threadpool-objec.patch Normal file
View File

@ -0,0 +1,148 @@
From 8eb37c4da70b875e1daf4976ec8624c26a2266db Mon Sep 17 00:00:00 2001
From: Sebastian Lackner <sebastian@fds-team.de>
Date: Sat, 20 Aug 2016 21:22:54 +0200
Subject: ntdll/tests: Add tests for releasing threadpool objects during
TpReleaseCleanupGroupMembers.
Signed-off-by: Sebastian Lackner <sebastian@fds-team.de>
---
dlls/ntdll/tests/threadpool.c | 109 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 108 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/tests/threadpool.c b/dlls/ntdll/tests/threadpool.c
index 9b4b522..7672d63 100644
--- a/dlls/ntdll/tests/threadpool.c
+++ b/dlls/ntdll/tests/threadpool.c
@@ -1,7 +1,7 @@
/*
* Unit test suite for thread pool functions
*
- * Copyright 2015 Sebastian Lackner
+ * Copyright 2015-2016 Sebastian Lackner
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -705,6 +705,112 @@ static void test_tp_work_scheduler(void)
pTpReleasePool(pool);
}
+static void CALLBACK work_release_cb(TP_CALLBACK_INSTANCE *instance, void *userdata, TP_WORK *work)
+{
+ HANDLE semaphore = userdata;
+ trace("Running work release callback\n");
+ ReleaseSemaphore(semaphore, 1, NULL);
+ Sleep(200); /* wait until main thread is in TpReleaseCleanupGroupMembers */
+ pTpReleaseWork(work);
+}
+
+static void CALLBACK timer_release_cb(TP_CALLBACK_INSTANCE *instance, void *userdata, TP_TIMER *timer)
+{
+ HANDLE semaphore = userdata;
+ trace("Running timer release callback\n");
+ ReleaseSemaphore(semaphore, 1, NULL);
+ Sleep(200); /* wait until main thread is in TpReleaseCleanupGroupMembers */
+ pTpReleaseTimer(timer);
+}
+
+static void CALLBACK wait_release_cb(TP_CALLBACK_INSTANCE *instance, void *userdata,
+ TP_WAIT *wait, TP_WAIT_RESULT result)
+{
+ HANDLE semaphore = userdata;
+ trace("Running wait release callback\n");
+ ReleaseSemaphore(semaphore, 1, NULL);
+ Sleep(200); /* wait until main thread is in TpReleaseCleanupGroupMembers */
+ pTpReleaseWait(wait);
+}
+
+static void test_tp_group_wait(void)
+{
+ TP_CALLBACK_ENVIRON environment;
+ TP_CLEANUP_GROUP *group;
+ LARGE_INTEGER when;
+ HANDLE semaphore;
+ NTSTATUS status;
+ TP_TIMER *timer;
+ TP_WAIT *wait;
+ TP_WORK *work;
+ TP_POOL *pool;
+ DWORD result;
+
+ semaphore = CreateSemaphoreA(NULL, 0, 1, NULL);
+ ok(semaphore != NULL, "CreateSemaphoreA failed %u\n", GetLastError());
+
+ /* allocate new threadpool */
+ pool = NULL;
+ status = pTpAllocPool(&pool, NULL);
+ ok(!status, "TpAllocPool failed with status %x\n", status);
+ ok(pool != NULL, "expected pool != NULL\n");
+
+ /* allocate a cleanup group */
+ group = NULL;
+ status = pTpAllocCleanupGroup(&group);
+ ok(!status, "TpAllocCleanupGroup failed with status %x\n", status);
+ ok(group != NULL, "expected pool != NULL\n");
+
+ /* release work object during TpReleaseCleanupGroupMembers */
+ work = NULL;
+ memset(&environment, 0, sizeof(environment));
+ environment.Version = 1;
+ environment.Pool = pool;
+ environment.CleanupGroup = group;
+ status = pTpAllocWork(&work, work_release_cb, semaphore, &environment);
+ ok(!status, "TpAllocWork failed with status %x\n", status);
+ ok(work != NULL, "expected work != NULL\n");
+ pTpPostWork(work);
+ result = WaitForSingleObject(semaphore, 1000);
+ ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result);
+ pTpReleaseCleanupGroupMembers(group, FALSE, NULL);
+
+ /* release timer object during TpReleaseCleanupGroupMembers */
+ timer = NULL;
+ memset(&environment, 0, sizeof(environment));
+ environment.Version = 1;
+ environment.Pool = pool;
+ environment.CleanupGroup = group;
+ status = pTpAllocTimer(&timer, timer_release_cb, semaphore, &environment);
+ ok(!status, "TpAllocTimer failed with status %x\n", status);
+ ok(timer != NULL, "expected timer != NULL\n");
+ when.QuadPart = 0;
+ pTpSetTimer(timer, &when, 0, 0);
+ result = WaitForSingleObject(semaphore, 1000);
+ ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result);
+ pTpReleaseCleanupGroupMembers(group, FALSE, NULL);
+
+ /* release wait object during TpReleaseCleanupGroupMembers */
+ wait = NULL;
+ memset(&environment, 0, sizeof(environment));
+ environment.Version = 1;
+ environment.Pool = pool;
+ environment.CleanupGroup = group;
+ status = pTpAllocWait(&wait, wait_release_cb, semaphore, &environment);
+ ok(!status, "TpAllocWait failed with status %x\n", status);
+ ok(wait != NULL, "expected wait != NULL\n");
+ when.QuadPart = 0;
+ pTpSetWait(wait, INVALID_HANDLE_VALUE, &when);
+ result = WaitForSingleObject(semaphore, 1000);
+ ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result);
+ pTpReleaseCleanupGroupMembers(group, FALSE, NULL);
+
+ /* cleanup */
+ pTpReleaseCleanupGroup(group);
+ pTpReleasePool(pool);
+ CloseHandle(semaphore);
+}
+
static DWORD group_cancel_tid;
static void CALLBACK group_cancel_cb(TP_CALLBACK_INSTANCE *instance, void *userdata)
@@ -1679,6 +1785,7 @@ START_TEST(threadpool)
test_tp_simple();
test_tp_work();
test_tp_work_scheduler();
+ test_tp_group_wait();
test_tp_group_cancel();
test_tp_instance();
test_tp_disassociate();
--
2.9.0

151
patches/ntdll-ThreadpoolCleanupGroup/0004-ntdll-Call-group-cancel-callback-with-the-correct-ar.patch Normal file
View File

@ -0,0 +1,151 @@
From 5ed40666bb3c8587a57e269e571f64cab27c20d4 Mon Sep 17 00:00:00 2001
From: Sebastian Lackner <sebastian@fds-team.de>
Date: Sat, 20 Aug 2016 21:23:24 +0200
Subject: ntdll: Call group cancel callback with the correct arguments.
Signed-off-by: Sebastian Lackner <sebastian@fds-team.de>
---
dlls/ntdll/tests/threadpool.c | 54 ++++++++++++++++++++++++++++++++++++-------
dlls/ntdll/threadpool.c | 7 +++---
2 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/dlls/ntdll/tests/threadpool.c b/dlls/ntdll/tests/threadpool.c
index 7672d63..f59a15c 100644
--- a/dlls/ntdll/tests/threadpool.c
+++ b/dlls/ntdll/tests/threadpool.c
@@ -813,21 +813,25 @@ static void test_tp_group_wait(void)
static DWORD group_cancel_tid;
-static void CALLBACK group_cancel_cb(TP_CALLBACK_INSTANCE *instance, void *userdata)
+static void CALLBACK simple_group_cancel_cb(TP_CALLBACK_INSTANCE *instance, void *userdata)
{
HANDLE *semaphores = userdata;
NTSTATUS status;
DWORD result;
+ int i;
- trace("Running group cancel callback\n");
+ trace("Running simple group cancel callback\n");
status = pTpCallbackMayRunLong(instance);
ok(status == STATUS_TOO_MANY_THREADS || broken(status == 1) /* Win Vista / 2008 */,
"expected STATUS_TOO_MANY_THREADS, got %08x\n", status);
ReleaseSemaphore(semaphores[1], 1, NULL);
- result = WaitForSingleObject(semaphores[0], 1000);
- ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result);
+ for (i = 0; i < 4; i++)
+ {
+ result = WaitForSingleObject(semaphores[0], 1000);
+ ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result);
+ }
ReleaseSemaphore(semaphores[1], 1, NULL);
}
@@ -836,6 +840,7 @@ static void CALLBACK group_cancel_cleanup_release_cb(void *object, void *userdat
HANDLE *semaphores = userdata;
trace("Running group cancel cleanup release callback\n");
group_cancel_tid = GetCurrentThreadId();
+ ok(object == (void *)0xdeadbeef, "expected 0xdeadbeef, got %p\n", object);
ReleaseSemaphore(semaphores[0], 1, NULL);
}
@@ -846,7 +851,23 @@ static void CALLBACK group_cancel_cleanup_increment_cb(void *object, void *userd
InterlockedIncrement((LONG *)userdata);
}
-static void CALLBACK unexpected_cb(TP_CALLBACK_INSTANCE *instance, void *userdata)
+static void CALLBACK unexpected_simple_cb(TP_CALLBACK_INSTANCE *instance, void *userdata)
+{
+ ok(0, "Unexpected callback\n");
+}
+
+static void CALLBACK unexpected_work_cb(TP_CALLBACK_INSTANCE *instance, void *userdata, TP_WORK *work)
+{
+ ok(0, "Unexpected callback\n");
+}
+
+static void CALLBACK unexpected_timer_cb(TP_CALLBACK_INSTANCE *instance, void *userdata, TP_TIMER *timer)
+{
+ ok(0, "Unexpected callback\n");
+}
+
+static void CALLBACK unexpected_wait_cb(TP_CALLBACK_INSTANCE *instance, void *userdata,
+ TP_WAIT *wait, TP_WAIT_RESULT result)
{
ok(0, "Unexpected callback\n");
}
@@ -858,12 +879,14 @@ static void test_tp_group_cancel(void)
LONG userdata, userdata2;
HANDLE semaphores[2];
NTSTATUS status;
+ TP_TIMER *timer;
+ TP_WAIT *wait;
TP_WORK *work;
TP_POOL *pool;
DWORD result;
int i;
- semaphores[0] = CreateSemaphoreA(NULL, 0, 1, NULL);
+ semaphores[0] = CreateSemaphoreA(NULL, 0, 4, NULL);
ok(semaphores[0] != NULL, "CreateSemaphoreA failed %u\n", GetLastError());
semaphores[1] = CreateSemaphoreA(NULL, 0, 1, NULL);
ok(semaphores[1] != NULL, "CreateSemaphoreA failed %u\n", GetLastError());
@@ -885,7 +908,7 @@ static void test_tp_group_cancel(void)
memset(&environment, 0, sizeof(environment));
environment.Version = 1;
environment.Pool = pool;
- status = pTpSimpleTryPost(group_cancel_cb, semaphores, &environment);
+ status = pTpSimpleTryPost(simple_group_cancel_cb, semaphores, &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);
@@ -895,9 +918,24 @@ static void test_tp_group_cancel(void)
environment.Pool = pool;
environment.CleanupGroup = group;
environment.CleanupGroupCancelCallback = group_cancel_cleanup_release_cb;
- status = pTpSimpleTryPost(unexpected_cb, NULL, &environment);
+ status = pTpSimpleTryPost(unexpected_simple_cb, (void *)0xdeadbeef, &environment);
ok(!status, "TpSimpleTryPost failed with status %x\n", status);
+ work = NULL;
+ status = pTpAllocWork(&work, unexpected_work_cb, (void *)0xdeadbeef, &environment);
+ ok(!status, "TpAllocWork failed with status %x\n", status);
+ ok(work != NULL, "expected work != NULL\n");
+
+ timer = NULL;
+ status = pTpAllocTimer(&timer, unexpected_timer_cb, (void *)0xdeadbeef, &environment);
+ ok(!status, "TpAllocTimer failed with status %x\n", status);
+ ok(timer != NULL, "expected timer != NULL\n");
+
+ wait = NULL;
+ status = pTpAllocWait(&wait, unexpected_wait_cb, (void *)0xdeadbeef, &environment);
+ ok(!status, "TpAllocWait failed with status %x\n", status);
+ ok(wait != NULL, "expected wait != NULL\n");
+
group_cancel_tid = 0xdeadbeef;
pTpReleaseCleanupGroupMembers(group, TRUE, semaphores);
result = WaitForSingleObject(semaphores[1], 1000);
diff --git a/dlls/ntdll/threadpool.c b/dlls/ntdll/threadpool.c
index eea5a88..61f6ea0 100644
--- a/dlls/ntdll/threadpool.c
+++ b/dlls/ntdll/threadpool.c
@@ -1966,10 +1966,11 @@ static void tp_object_cancel( struct threadpool_object *object, BOOL group_cance
RtlLeaveCriticalSection( &pool->cs );
/* Execute group cancellation callback if defined, and if this was actually a group cancel. */
- if (pending_callbacks && group_cancel && object->group_cancel_callback)
+ if (group_cancel && object->group_cancel_callback)
{
- TRACE( "executing group cancel callback %p(%p, %p)\n", object->group_cancel_callback, object, userdata );
- object->group_cancel_callback( object, userdata );
+ 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 );
}
--
2.9.0

193
patches/ntdll-ThreadpoolCleanupGroup/0005-ntdll-Group-cancel-callbacks-should-be-executed-afte.patch Normal file
View File

@ -0,0 +1,193 @@
From 0a7918f7b5703de00508a54baf82a6423e0a04f5 Mon Sep 17 00:00:00 2001
From: Sebastian Lackner <sebastian@fds-team.de>
Date: Sat, 20 Aug 2016 21:23:48 +0200
Subject: ntdll: Group cancel callbacks should be executed after waiting for
pending callbacks.
Signed-off-by: Sebastian Lackner <sebastian@fds-team.de>
---
dlls/ntdll/tests/threadpool.c | 65 +++++++++++++++++++++++++++++++++++++++++++
dlls/ntdll/threadpool.c | 29 +++++++++----------
2 files changed, 80 insertions(+), 14 deletions(-)
diff --git a/dlls/ntdll/tests/threadpool.c b/dlls/ntdll/tests/threadpool.c
index f59a15c..7eccb23 100644
--- a/dlls/ntdll/tests/threadpool.c
+++ b/dlls/ntdll/tests/threadpool.c
@@ -835,6 +835,18 @@ static void CALLBACK simple_group_cancel_cb(TP_CALLBACK_INSTANCE *instance, void
ReleaseSemaphore(semaphores[1], 1, NULL);
}
+static void CALLBACK work_group_cancel_cb(TP_CALLBACK_INSTANCE *instance, void *userdata, TP_WORK *work)
+{
+ HANDLE *semaphores = userdata;
+ DWORD result;
+
+ trace("Running work group cancel callback\n");
+
+ ReleaseSemaphore(semaphores[1], 1, NULL);
+ result = WaitForSingleObject(semaphores[0], 200);
+ ok(result == WAIT_TIMEOUT, "WaitForSingleObject returned %u\n", result);
+}
+
static void CALLBACK group_cancel_cleanup_release_cb(void *object, void *userdata)
{
HANDLE *semaphores = userdata;
@@ -844,6 +856,15 @@ static void CALLBACK group_cancel_cleanup_release_cb(void *object, void *userdat
ReleaseSemaphore(semaphores[0], 1, NULL);
}
+static void CALLBACK group_cancel_cleanup_release2_cb(void *object, void *userdata)
+{
+ HANDLE *semaphores = userdata;
+ trace("Running group cancel cleanup release2 callback\n");
+ group_cancel_tid = GetCurrentThreadId();
+ ok(object == userdata, "expected %p, got %p\n", userdata, object);
+ ReleaseSemaphore(semaphores[0], 1, NULL);
+}
+
static void CALLBACK group_cancel_cleanup_increment_cb(void *object, void *userdata)
{
trace("Running group cancel cleanup increment callback\n");
@@ -872,6 +893,11 @@ static void CALLBACK unexpected_wait_cb(TP_CALLBACK_INSTANCE *instance, void *us
ok(0, "Unexpected callback\n");
}
+static void CALLBACK unexpected_group_cancel_cleanup_cb(void *object, void *userdata)
+{
+ ok(0, "Unexpected callback\n");
+}
+
static void test_tp_group_cancel(void)
{
TP_CALLBACK_ENVIRON environment;
@@ -943,6 +969,45 @@ static void test_tp_group_cancel(void)
ok(group_cancel_tid == GetCurrentThreadId(), "expected tid %x, got %x\n",
GetCurrentThreadId(), group_cancel_tid);
+ /* test if cancellation callbacks are executed before or after wait */
+ work = NULL;
+ memset(&environment, 0, sizeof(environment));
+ environment.Version = 1;
+ environment.Pool = pool;
+ environment.CleanupGroup = group;
+ environment.CleanupGroupCancelCallback = group_cancel_cleanup_release2_cb;
+ status = pTpAllocWork(&work, work_group_cancel_cb, semaphores, &environment);
+ ok(!status, "TpAllocWork failed with status %x\n", status);
+ ok(work != NULL, "expected work != NULL\n");
+ pTpPostWork(work);
+ pTpPostWork(work);
+
+ result = WaitForSingleObject(semaphores[1], 1000);
+ ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result);
+
+ group_cancel_tid = 0xdeadbeef;
+ pTpReleaseCleanupGroupMembers(group, TRUE, semaphores);
+ result = WaitForSingleObject(semaphores[0], 1000);
+ ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result);
+ ok(group_cancel_tid == GetCurrentThreadId(), "expected tid %x, got %x\n",
+ GetCurrentThreadId(), group_cancel_tid);
+
+ /* group cancel callback is not executed if object is destroyed while waiting */
+ work = NULL;
+ memset(&environment, 0, sizeof(environment));
+ environment.Version = 1;
+ environment.Pool = pool;
+ environment.CleanupGroup = group;
+ environment.CleanupGroupCancelCallback = unexpected_group_cancel_cleanup_cb;
+ status = pTpAllocWork(&work, work_release_cb, semaphores[1], &environment);
+ ok(!status, "TpAllocWork failed with status %x\n", status);
+ ok(work != NULL, "expected work != NULL\n");
+ pTpPostWork(work);
+
+ result = WaitForSingleObject(semaphores[1], 1000);
+ ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result);
+ pTpReleaseCleanupGroupMembers(group, TRUE, NULL);
+
/* 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 61f6ea0..a572869 100644
--- a/dlls/ntdll/threadpool.c
+++ b/dlls/ntdll/threadpool.c
@@ -1948,7 +1948,7 @@ static void tp_object_submit( struct threadpool_object *object, BOOL signaled )
*
* Cancels all currently pending callbacks for a specific object.
*/
-static void tp_object_cancel( struct threadpool_object *object, BOOL group_cancel, PVOID userdata )
+static void tp_object_cancel( struct threadpool_object *object )
{
struct threadpool *pool = object->pool;
LONG pending_callbacks = 0;
@@ -1965,15 +1965,6 @@ static void tp_object_cancel( struct threadpool_object *object, BOOL group_cance
}
RtlLeaveCriticalSection( &pool->cs );
- /* Execute group cancellation callback if defined, and if this was actually a group cancel. */
- if (group_cancel && 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 );
- }
-
while (pending_callbacks--)
tp_object_release( object );
}
@@ -2598,7 +2589,7 @@ VOID WINAPI TpReleaseCleanupGroupMembers( TP_CLEANUP_GROUP *group, BOOL cancel_p
{
LIST_FOR_EACH_ENTRY( object, &members, struct threadpool_object, group_entry )
{
- tp_object_cancel( object, TRUE, userdata );
+ tp_object_cancel( object );
}
}
@@ -2607,6 +2598,16 @@ 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)
+ {
+ 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 );
@@ -2900,7 +2901,7 @@ VOID WINAPI TpWaitForTimer( TP_TIMER *timer, BOOL cancel_pending )
TRACE( "%p %d\n", timer, cancel_pending );
if (cancel_pending)
- tp_object_cancel( this, FALSE, NULL );
+ tp_object_cancel( this );
tp_object_wait( this, FALSE );
}
@@ -2914,7 +2915,7 @@ VOID WINAPI TpWaitForWait( TP_WAIT *wait, BOOL cancel_pending )
TRACE( "%p %d\n", wait, cancel_pending );
if (cancel_pending)
- tp_object_cancel( this, FALSE, NULL );
+ tp_object_cancel( this );
tp_object_wait( this, FALSE );
}
@@ -2928,6 +2929,6 @@ VOID WINAPI TpWaitForWork( TP_WORK *work, BOOL cancel_pending )
TRACE( "%p %u\n", work, cancel_pending );
if (cancel_pending)
- tp_object_cancel( this, FALSE, NULL );
+ tp_object_cancel( this );
tp_object_wait( this, FALSE );
}
--
2.9.0

114
patches/ntdll-ThreadpoolCleanupGroup/0006-ntdll-Do-not-call-group-cancel-callback-for-finished.patch Normal file
View File

@ -0,0 +1,114 @@
From 0415a5cdbebd81532008f059a163cd18da2ddadb Mon Sep 17 00:00:00 2001
From: Sebastian Lackner <sebastian@fds-team.de>
Date: Sat, 20 Aug 2016 22:38:43 +0200
Subject: ntdll: Do not call group cancel callback for finished simple
callbacks. (v2)
Signed-off-by: Sebastian Lackner <sebastian@fds-team.de>
---
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

76
patches/ntdll-ThreadpoolCleanupGroup/0007-services-Remove-synchronization-for-CloseThreadpoolC.patch Normal file
View File

@ -0,0 +1,76 @@
From 6746c0e13d0536a3463ac05c4c6ca5124f3ee4cd Mon Sep 17 00:00:00 2001
From: Sebastian Lackner <sebastian@fds-team.de>
Date: Sat, 20 Aug 2016 21:24:52 +0200
Subject: services: Remove synchronization for
CloseThreadpoolCleanupGroupMembers.
Signed-off-by: Sebastian Lackner <sebastian@fds-team.de>
---
programs/services/rpc.c | 28 ++++++++--------------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/programs/services/rpc.c b/programs/services/rpc.c
index 688e4a0..bd6af95 100644
--- a/programs/services/rpc.c
+++ b/programs/services/rpc.c
@@ -85,30 +85,22 @@ struct sc_lock
struct scmdatabase *db;
};
-static CRITICAL_SECTION shutdown_cs;
-static CRITICAL_SECTION_DEBUG critsect_debug =
-{
- 0, 0, &shutdown_cs,
- { &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList },
- 0, 0, { (DWORD_PTR)(__FILE__ ": shutdown_cs") }
-};
-static CRITICAL_SECTION shutdown_cs = { &critsect_debug, -1, 0, 0, 0, 0 };
-static BOOL service_shutdown;
-
static PTP_CLEANUP_GROUP cleanup_group;
HANDLE exit_event;
+static void CALLBACK group_cancel_callback(void *object, void *userdata)
+{
+ struct process_entry *process = object;
+ release_process(process);
+}
+
static void CALLBACK terminate_callback(TP_CALLBACK_INSTANCE *instance, void *context,
TP_WAIT *wait, TP_WAIT_RESULT result)
{
struct process_entry *process = context;
if (result == WAIT_TIMEOUT) process_terminate(process);
release_process(process);
-
- /* synchronize with CloseThreadpoolCleanupGroupMembers */
- EnterCriticalSection(&shutdown_cs);
- if (!service_shutdown) CloseThreadpoolWait(wait);
- LeaveCriticalSection(&shutdown_cs);
+ CloseThreadpoolWait(wait);
}
static void terminate_after_timeout(struct process_entry *process, DWORD timeout)
@@ -121,6 +113,7 @@ static void terminate_after_timeout(struct process_entry *process, DWORD timeout
memset(&environment, 0, sizeof(environment));
environment.Version = 1;
environment.CleanupGroup = cleanup_group;
+ environment.CleanupGroupCancelCallback = group_cancel_callback;
timestamp.QuadPart = (ULONGLONG)timeout * -10000;
ft.dwLowDateTime = timestamp.u.LowPart;
@@ -1914,11 +1907,6 @@ void RPC_Stop(void)
RpcMgmtStopServerListening(NULL);
RpcServerUnregisterIf(svcctl_v2_0_s_ifspec, NULL, TRUE);
- /* synchronize with CloseThreadpoolWait */
- EnterCriticalSection(&shutdown_cs);
- service_shutdown = TRUE;
- LeaveCriticalSection(&shutdown_cs);
-
CloseThreadpoolCleanupGroupMembers(cleanup_group, TRUE, NULL);
CloseThreadpoolCleanupGroup(cleanup_group);
CloseHandle(exit_event);
--
2.9.0

1
patches/ntdll-ThreadpoolCleanupGroup/definition Normal file
View File

@ -0,0 +1 @@
Fixes: Fix multiple bugs related to threadpool cleanup groups and cancel callbacks

28
patches/patchinstall.sh
View File

@ -238,6 +238,7 @@ patch_enable_all ()
enable_ntdll_SystemRoot_Symlink="$1"
enable_ntdll_ThreadTime="$1"
enable_ntdll_Threading="$1"
enable_ntdll_ThreadpoolCleanupGroup="$1"
enable_ntdll_User_Shared_Data="$1"
enable_ntdll_WRITECOPY="$1"
enable_ntdll_Wait_User_APC="$1"
@ -885,6 +886,9 @@ patch_enable ()
ntdll-Threading)
enable_ntdll_Threading="$2"
;;
ntdll-ThreadpoolCleanupGroup)
enable_ntdll_ThreadpoolCleanupGroup="$2"
;;
ntdll-User_Shared_Data)
enable_ntdll_User_Shared_Data="$2"
;;
@ -5218,6 +5222,30 @@ if test "$enable_ntdll_Threading" -eq 1; then
) >> "$patchlist"
fi
# Patchset ntdll-ThreadpoolCleanupGroup
# |
# | Modified files:
# | * dlls/ntdll/tests/threadpool.c, dlls/ntdll/threadpool.c, programs/services/rpc.c
# |
if test "$enable_ntdll_ThreadpoolCleanupGroup" -eq 1; then
patch_apply ntdll-ThreadpoolCleanupGroup/0001-ntdll-tests-Use-longer-waits-to-reduce-risk-of-rando.patch
patch_apply ntdll-ThreadpoolCleanupGroup/0002-ntdll-Allow-to-release-threadpool-objects-while-wait.patch
patch_apply ntdll-ThreadpoolCleanupGroup/0003-ntdll-tests-Add-tests-for-releasing-threadpool-objec.patch
patch_apply ntdll-ThreadpoolCleanupGroup/0004-ntdll-Call-group-cancel-callback-with-the-correct-ar.patch
patch_apply ntdll-ThreadpoolCleanupGroup/0005-ntdll-Group-cancel-callbacks-should-be-executed-afte.patch
patch_apply ntdll-ThreadpoolCleanupGroup/0006-ntdll-Do-not-call-group-cancel-callback-for-finished.patch
patch_apply ntdll-ThreadpoolCleanupGroup/0007-services-Remove-synchronization-for-CloseThreadpoolC.patch
(
echo '+ { "Sebastian Lackner", "ntdll/tests: Use longer waits to reduce risk of random failures on the testbot.", 1 },';
echo '+ { "Sebastian Lackner", "ntdll: Allow to release threadpool objects while waiting for group.", 1 },';
echo '+ { "Sebastian Lackner", "ntdll/tests: Add tests for releasing threadpool objects during TpReleaseCleanupGroupMembers.", 1 },';
echo '+ { "Sebastian Lackner", "ntdll: Call group cancel callback with the correct arguments.", 1 },';
echo '+ { "Sebastian Lackner", "ntdll: Group cancel callbacks should be executed after waiting for pending callbacks.", 1 },';
echo '+ { "Sebastian Lackner", "ntdll: Do not call group cancel callback for finished simple callbacks.", 2 },';
echo '+ { "Sebastian Lackner", "services: Remove synchronization for CloseThreadpoolCleanupGroupMembers.", 1 },';
) >> "$patchlist"
fi
# Patchset ntdll-User_Shared_Data
# |
# | Modified files: