[Python-checkins] bpo-31249: Fix ref cycle in ThreadPoolExecutor (#3253)

[Victor Stinner]

https://github.com/python/cpython/commit/60f3f1fb5cbf5354c3081138be806c56cb04153f
commit: 60f3f1fb5cbf5354c3081138be806c56cb04153f
branch: 3.6
author: Victor Stinner <victor.stinner at gmail.com>
committer: GitHub <noreply at github.com>
date: 2017-09-01T14:46:44+02:00
summary:

bpo-31249: Fix ref cycle in ThreadPoolExecutor (#3253)

[3.6] bpo-31249: Fix ref cycle in ThreadPoolExecutor

files:
A Misc/NEWS.d/next/Library/2017-08-22-12-44-48.bpo-31249.STPbb9.rst
M Lib/concurrent/futures/thread.py
M Lib/test/test_concurrent_futures.py

diff --git a/Lib/concurrent/futures/thread.py b/Lib/concurrent/futures/thread.py
index 03d276b63f6..5ade790e3db 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/Lib/test/test_concurrent_futures.py b/Lib/test/test_concurrent_futures.py
index 92a3ebdd886..90dec61814c 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):
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 00000000000..f11a66802d1
--- /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.