From f55d496a6be3184d96e88fe501615756421aca45 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 21 Aug 2017 23:24:24 +0200 Subject: [PATCH 1/2] bpo-31249: test_concurrent_futures checks dangling threads (#3167) Add a BaseTestCase class to test_concurrent_futures to check for dangling threads and processes on all tests, not only tests using ExecutorMixin. (cherry picked from commit 489d91c61d02f401551966f1558ee2dc87fc0ad9) --- Lib/test/test_concurrent_futures.py | 32 ++++++++++++++++++----------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_concurrent_futures.py b/Lib/test/test_concurrent_futures.py index 92a3ebdd886c126..90dec61814cac6a 100644 --- a/Lib/test/test_concurrent_futures.py +++ b/Lib/test/test_concurrent_futures.py @@ -59,11 +59,20 @@ def my_method(self): pass +class BaseTestCase(unittest.TestCase): + def setUp(self): + self._thread_key = test.support.threading_setup() + + def tearDown(self): + test.support.reap_children() + test.support.threading_cleanup(*self._thread_key) + + class ExecutorMixin: worker_count = 5 def setUp(self): - self._thread_cleanup = test.support.threading_setup() + super().setUp() self.t1 = time.time() try: @@ -81,8 +90,7 @@ def tearDown(self): print("%.2fs" % dt, end=' ') self.assertLess(dt, 60, "synchronization issue: test lasted too long") - test.support.threading_cleanup(*self._thread_cleanup) - test.support.reap_children() + super().tearDown() def _prime_executor(self): # Make sure that the executor is ready to do work before running the @@ -130,7 +138,7 @@ def test_hang_issue12364(self): f.result() -class ThreadPoolShutdownTest(ThreadPoolMixin, ExecutorShutdownTest, unittest.TestCase): +class ThreadPoolShutdownTest(ThreadPoolMixin, ExecutorShutdownTest, BaseTestCase): def _prime_executor(self): pass @@ -186,7 +194,7 @@ def test_thread_names_default(self): t.join() -class ProcessPoolShutdownTest(ProcessPoolMixin, ExecutorShutdownTest, unittest.TestCase): +class ProcessPoolShutdownTest(ProcessPoolMixin, ExecutorShutdownTest, BaseTestCase): def _prime_executor(self): pass @@ -323,7 +331,7 @@ def test_timeout(self): self.assertEqual(set([future2]), pending) -class ThreadPoolWaitTests(ThreadPoolMixin, WaitTests, unittest.TestCase): +class ThreadPoolWaitTests(ThreadPoolMixin, WaitTests, BaseTestCase): def test_pending_calls_race(self): # Issue #14406: multi-threaded race condition when waiting on all @@ -341,7 +349,7 @@ def future_func(): sys.setswitchinterval(oldswitchinterval) -class ProcessPoolWaitTests(ProcessPoolMixin, WaitTests, unittest.TestCase): +class ProcessPoolWaitTests(ProcessPoolMixin, WaitTests, BaseTestCase): pass @@ -390,11 +398,11 @@ def test_duplicate_futures(self): self.assertEqual(len(completed), 1) -class ThreadPoolAsCompletedTests(ThreadPoolMixin, AsCompletedTests, unittest.TestCase): +class ThreadPoolAsCompletedTests(ThreadPoolMixin, AsCompletedTests, BaseTestCase): pass -class ProcessPoolAsCompletedTests(ProcessPoolMixin, AsCompletedTests, unittest.TestCase): +class ProcessPoolAsCompletedTests(ProcessPoolMixin, AsCompletedTests, BaseTestCase): pass @@ -465,7 +473,7 @@ def test_max_workers_negative(self): self.executor_type(max_workers=number) -class ThreadPoolExecutorTest(ThreadPoolMixin, ExecutorTest, unittest.TestCase): +class ThreadPoolExecutorTest(ThreadPoolMixin, ExecutorTest, BaseTestCase): def test_map_submits_without_iteration(self): """Tests verifying issue 11777.""" finished = [] @@ -482,7 +490,7 @@ def test_default_workers(self): (os.cpu_count() or 1) * 5) -class ProcessPoolExecutorTest(ProcessPoolMixin, ExecutorTest, unittest.TestCase): +class ProcessPoolExecutorTest(ProcessPoolMixin, ExecutorTest, BaseTestCase): def test_killed_child(self): # When a child process is abruptly terminated, the whole pool gets # "broken". @@ -538,7 +546,7 @@ def test_traceback(self): f1.getvalue()) -class FutureTests(unittest.TestCase): +class FutureTests(BaseTestCase): def test_done_callback_with_result(self): callback_result = None def fn(callback_future): From d5ae2ec73ea20e47a8720cd13f67ffee19a767c5 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 22 Aug 2017 16:50:42 +0200 Subject: [PATCH 2/2] bpo-31249: Fix ref cycle in ThreadPoolExecutor (#3178) * bpo-31249: Fix ref cycle in ThreadPoolExecutor concurrent.futures: WorkItem.run() used by ThreadPoolExecutor now breaks a reference cycle between an exception object and the WorkItem object. ThreadPoolExecutor.shutdown() now also clears its threads set. * shutdown() now only clears threads if wait is true. * Revert changes on shutdown() (cherry picked from commit bc61315377056fe362b744d9c44e17cd3178ce54) --- Lib/concurrent/futures/thread.py | 6 ++++-- .../next/Library/2017-08-22-12-44-48.bpo-31249.STPbb9.rst | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-08-22-12-44-48.bpo-31249.STPbb9.rst diff --git a/Lib/concurrent/futures/thread.py b/Lib/concurrent/futures/thread.py index 03d276b63f63cae..5ade790e3db4100 100644 --- a/Lib/concurrent/futures/thread.py +++ b/Lib/concurrent/futures/thread.py @@ -53,8 +53,10 @@ def run(self): try: result = self.fn(*self.args, **self.kwargs) - except BaseException as e: - self.future.set_exception(e) + except BaseException as exc: + self.future.set_exception(exc) + # Break a reference cycle with the exception 'exc' + self = None else: self.future.set_result(result) diff --git a/Misc/NEWS.d/next/Library/2017-08-22-12-44-48.bpo-31249.STPbb9.rst b/Misc/NEWS.d/next/Library/2017-08-22-12-44-48.bpo-31249.STPbb9.rst new file mode 100644 index 000000000000000..f11a66802d1bc14 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-08-22-12-44-48.bpo-31249.STPbb9.rst @@ -0,0 +1,2 @@ +concurrent.futures: WorkItem.run() used by ThreadPoolExecutor now breaks a +reference cycle between an exception object and the WorkItem object.