Skip to content

Commit d67b22c

Browse files
committed
Some nits. Improve wordings in docstrings and comments, and avoid relying on
sys.getrefcount() in tests.
1 parent d581bba commit d67b22c

File tree

4 files changed

+33
-22
lines changed

4 files changed

+33
-22
lines changed

Lib/concurrent/futures/_base.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,15 @@ def _create_and_install_waiters(fs, return_when):
172172

173173

174174
def _yield_and_decref(fs, ref_collect):
175-
"""Yields a future. Before yielding, removes the future
176-
from each set in collection of sets (`ref_collect`)."""
175+
"""
176+
Iterate on the list *fs*, yielding objects one by one in reverse order.
177+
Before yielding an object, it is removed from each set in
178+
the collection of sets *ref_collect*.
179+
"""
177180
while fs:
178181
for futures_set in ref_collect:
179182
futures_set.remove(fs[-1])
183+
# Careful not to keep a reference to the popped value
180184
yield fs.pop()
181185

182186

@@ -566,6 +570,7 @@ def result_iterator():
566570
# reverse to keep finishing order
567571
fs.reverse()
568572
while fs:
573+
# Careful not to keep a reference to the popped future
569574
if timeout is None:
570575
yield fs.pop().result()
571576
else:

Lib/concurrent/futures/process.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -357,12 +357,12 @@ def _check_system_limits():
357357
raise NotImplementedError(_system_limited)
358358

359359

360-
def _chain_from_iterable(iterable):
360+
def _chain_from_iterable_of_lists(iterable):
361361
"""
362-
Different implementation of itertools.chain.from_iterable.
363-
The difference is _chain_from_iterable do not keep reference to returned objects.
362+
Specialized implementation of itertools.chain.from_iterable.
363+
Each item in *iterable* should be a list. This function is
364+
careful not to keep references to yielded objects.
364365
"""
365-
366366
for element in iterable:
367367
element.reverse()
368368
while element:
@@ -494,7 +494,7 @@ def map(self, fn, *iterables, timeout=None, chunksize=1):
494494
results = super().map(partial(_process_chunk, fn),
495495
_get_chunks(*iterables, chunksize=chunksize),
496496
timeout=timeout)
497-
return _chain_from_iterable(results)
497+
return _chain_from_iterable_of_lists(results)
498498

499499
def shutdown(self, wait=True):
500500
with self._shutdown_lock:

Lib/test/test_concurrent_futures.py

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,15 @@ def sleep_and_print(t, msg):
5454
sys.stdout.flush()
5555

5656

57-
def _dummy_object_fn(_):
58-
return object()
59-
60-
6157
class MyObject(object):
6258
def my_method(self):
6359
pass
6460

6561

62+
def make_dummy_object(_):
63+
return MyObject()
64+
65+
6666
class BaseTestCase(unittest.TestCase):
6767
def setUp(self):
6868
self._thread_key = test.support.threading_setup()
@@ -401,27 +401,31 @@ def test_duplicate_futures(self):
401401
self.assertEqual(len(completed), 1)
402402

403403
def test_free_reference_yielded_future(self):
404-
# Issue #14406: Generator should not keep reference
405-
# for finished futures.
404+
# Issue #14406: Generator should not keep references
405+
# to finished futures.
406406
futures_list = [Future() for _ in range(8)]
407407
futures_list.append(create_future(state=CANCELLED_AND_NOTIFIED))
408408
futures_list.append(create_future(state=SUCCESSFUL_FUTURE))
409409

410410
with self.assertRaises(futures.TimeoutError):
411411
for future in futures.as_completed(futures_list, timeout=0):
412412
futures_list.remove(future)
413-
self.assertEqual(sys.getrefcount(future), 2)
413+
wr = weakref.ref(future)
414+
del future
415+
self.assertIsNone(wr())
414416

415417
futures_list[0].set_result("test")
416418
for future in futures.as_completed(futures_list):
417419
futures_list.remove(future)
418-
self.assertEqual(sys.getrefcount(future), 2)
420+
wr = weakref.ref(future)
421+
del future
422+
self.assertIsNone(wr())
419423
if futures_list:
420424
futures_list[0].set_result("test")
421425

422426
def test_correct_timeout_exception_msg(self):
423427
futures_list = [CANCELLED_AND_NOTIFIED_FUTURE, PENDING_FUTURE,
424-
RUNNING_FUTURE, SUCCESSFUL_FUTURE]
428+
RUNNING_FUTURE, SUCCESSFUL_FUTURE]
425429

426430
with self.assertRaises(futures.TimeoutError) as cm:
427431
list(futures.as_completed(futures_list, timeout=0))
@@ -508,10 +512,12 @@ def test_max_workers_negative(self):
508512
self.executor_type(max_workers=number)
509513

510514
def test_free_reference(self):
511-
# Issue #14406: Result iterator should not keep reference
512-
# for finished futures.
513-
for result_object in self.executor.map(_dummy_object_fn, range(10)):
514-
self.assertEqual(sys.getrefcount(result_object), 2)
515+
# Issue #14406: Result iterator should not keep an internal
516+
# reference to result objects.
517+
for obj in self.executor.map(make_dummy_object, range(10)):
518+
wr = weakref.ref(obj)
519+
del obj
520+
self.assertIsNone(wr())
515521

516522

517523
class ThreadPoolExecutorTest(ThreadPoolMixin, ExecutorTest, BaseTestCase):
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
concurrent.futures as_complie and map iterators do not keep reference to
2-
returned object
1+
The ``map()`` and ``as_completed()`` iterators in ``concurrent.futures``
2+
now avoid keeping a reference to yielded objects.

0 commit comments

Comments
 (0)