classification
Title: concurrent.futures does not validate that max_workers is proper
Type: behavior Stage: commit review
Components: Library (Lib) Versions: Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: bquinlan Nosy List: Claudiu.Popa, Jim.Jewett, bquinlan, flox, python-dev
Priority: normal Keywords: patch

Created on 2014-04-27 11:39 by Claudiu.Popa, last changed 2014-05-17 20:53 by bquinlan. This issue is now closed.

Files
File name Uploaded Description Edit
futures_max_workers.patch Claudiu.Popa, 2014-04-27 11:39 review
issue21362.patch Claudiu.Popa, 2014-04-27 16:30 review
issue21362_1.patch Claudiu.Popa, 2014-04-27 17:00 Fix typo, don't duplicate the test. review
Messages (7)
msg217258 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-04-27 11:39
Due to some bad math on my side, I passed max_workers=0 to concurrent.futures.ThreadPoolExecutor. This didn't fail properly, but hanged. The same behaviour occurs in ProcessPoolExecutor, but this time it fails internally with something like this:

Exception in thread Thread-1:
Traceback (most recent call last):
  File "C:\Python34\lib\threading.py", line 921, in _bootstrap_inner
    self.run()
  File "C:\Python34\lib\threading.py", line 869, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Python34\lib\concurrent\futures\process.py", line 225, in _queue_management_worker
    assert sentinels
AssertionError

The attached patch checks that *max_workers* is <= 0 and raises ValueError if so.
msg217259 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-04-27 11:41
For instance, multiprocessing behaves like this:

>>> multiprocessing.Pool(-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python34\lib\multiprocessing\context.py", line 118, in Pool
    context=self.get_context())
  File "C:\Python34\lib\multiprocessing\pool.py", line 157, in __init__
    raise ValueError("Number of processes must be at least 1")
ValueError: Number of processes must be at least 1
>>>
msg217280 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-04-27 16:30
Attached patch with improvements suggested by Charles-François Natali. Thank you for the review.
msg217376 - (view) Author: Jim Jewett (Jim.Jewett) (Python triager) Date: 2014-04-28 15:01
I confirm the bug.
The patch looks good.
msg217532 - (view) Author: Brian Quinlan (bquinlan) * (Python committer) Date: 2014-04-29 17:50
The patch looks good to me too. I'll commit it soon.
msg218714 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-05-17 20:52
New changeset 3024ad49f00e by Brian Quinlan in branch 'default':
Issue #21362: concurrent.futures does not validate that max_workers is proper
http://hg.python.org/cpython/rev/3024ad49f00e
msg218715 - (view) Author: Brian Quinlan (bquinlan) * (Python committer) Date: 2014-05-17 20:53
Committed, thanks for the fix!
History
Date User Action Args
2014-05-17 20:53:32bquinlansetstatus: open -> closed
resolution: fixed
messages: + msg218715
2014-05-17 20:52:35python-devsetnosy: + python-dev
messages: + msg218714
2014-04-29 17:50:38bquinlansetassignee: bquinlan
messages: + msg217532
2014-04-28 15:01:51Jim.Jewettsetnosy: + Jim.Jewett
messages: + msg217376
2014-04-27 19:30:49neologixsetstage: patch review -> commit review
2014-04-27 17:00:21Claudiu.Popasetfiles: + issue21362_1.patch
2014-04-27 16:57:10floxsetnosy: + flox
stage: patch review

versions: + Python 3.4
2014-04-27 16:30:17Claudiu.Popasetfiles: + issue21362.patch

messages: + msg217280
2014-04-27 11:41:04Claudiu.Popasetmessages: + msg217259
2014-04-27 11:39:29Claudiu.Popacreate