westonpace commented on a change in pull request #10401:
URL: https://github.com/apache/arrow/pull/10401#discussion_r648543567
##########
File path: cpp/src/arrow/util/thread_pool_test.cc
##########
@@ -452,6 +454,42 @@ TEST_F(TestThreadPool, QuickShutdown) {
add_tester.CheckNotAllComputed();
}
+TEST_F(TestThreadPool, DestroyWithoutShutdown) {
+ std::mutex mx;
+ std::condition_variable cv;
+ std::weak_ptr<ThreadPool> weak_pool;
+ bool ready = false;
+ bool completed = false;
+ {
+ auto pool = this->MakeThreadPool(1);
+ weak_pool = pool;
+ // Simulate Windows
+ pool->shutdown_on_destroy_ = false;
+
+ ASSERT_OK(pool->Spawn([&mx, &cv, &completed, &ready] {
+ std::unique_lock<std::mutex> lock(mx);
+ ready = true;
+ cv.notify_one();
+ cv.wait(lock);
+ completed = true;
+ }));
+ }
+
+ std::unique_lock<std::mutex> lock(mx);
+ cv.wait(lock, [&ready] { return ready; });
+ // Thread pool has shut down, now we unblock the task
+ cv.notify_one();
+ lock.unlock();
+
+ // The worker thread should be able to keep the thread pool alive by itself
Review comment:
It's more of an implementation detail (since the worker thread calls
methods on the thread pool). You are right that it doesn't really exist as a
"black box" requirement. The thread pool user shouldn't care.
I can take out the assert. If the worker thread were doing the wrong thing
it would segfault and/or throw an exception which should cause the test to fail
so there may not be anything I need to assert.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
