-
-
Notifications
You must be signed in to change notification settings - Fork 35k
bpo-5001: More-informative multiprocessing error messages #3079
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
23f0559
09e2391
e8b45b6
1fa907e
6344adc
3251312
cfae761
c0fb430
fa446ae
f90e227
8220864
70cd790
ae7a107
f29c97f
fb3fb93
58a5a8e
4bda162
04f0a44
c96980d
01f21fd
4a7558b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -211,7 +211,10 @@ def free(self, block): | |
| # synchronously sometimes later from malloc() or free(), by calling | ||
| # _free_pending_blocks() (appending and retrieving from a list is not | ||
| # strictly thread-safe but under cPython it's atomic thanks to the GIL). | ||
| assert os.getpid() == self._lastpid | ||
| if os.getpid() != self._lastpid: | ||
| raise ValueError( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Neither this one.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto to the above.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ow. I should have been more explicit because now I don't remember what I meant here. Sorry :-(
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As an absent-minded professor, I understand fully :-} |
||
| "My pid ({0:n}) is not last pid {1:n}".format( | ||
| os.getpid(),self._lastpid)) | ||
| if not self._lock.acquire(False): | ||
| # can't acquire the lock right now, add the block to the list of | ||
| # pending blocks to free | ||
|
|
@@ -227,7 +230,10 @@ def free(self, block): | |
|
|
||
| def malloc(self, size): | ||
| # return a block of right size (possibly rounded up) | ||
| assert 0 <= size < sys.maxsize | ||
| if size < 0: | ||
| raise ValueError("Size {0:n} out of range".format(size)) | ||
| if sys.maxsize <= size: | ||
| raise OverflowError("Size {0:n} too large".format(size)) | ||
| if os.getpid() != self._lastpid: | ||
| self.__init__() # reinitialize after fork | ||
| with self._lock: | ||
|
|
@@ -250,7 +256,10 @@ class BufferWrapper(object): | |
| _heap = Heap() | ||
|
|
||
| def __init__(self, size): | ||
| assert 0 <= size < sys.maxsize | ||
| if size < 0: | ||
| raise ValueError("Size {0:n} out of range".format(size)) | ||
| if sys.maxsize <= size: | ||
| raise OverflowError("Size {0:n} too large".format(size)) | ||
| block = BufferWrapper._heap.malloc(size) | ||
| self._state = (block, size) | ||
| util.Finalize(self, BufferWrapper._heap.free, args=(block,)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,7 +92,9 @@ def __repr__(self): | |
|
|
||
| def worker(inqueue, outqueue, initializer=None, initargs=(), maxtasks=None, | ||
| wrap_exception=False): | ||
| assert maxtasks is None or (type(maxtasks) == int and maxtasks > 0) | ||
| if (maxtasks is not None) and not (isinstance(maxtasks, int) | ||
| and maxtasks >= 1): | ||
| raise AssertionError("Maxtasks {!r} is not valid".format(maxtasks)) | ||
| put = outqueue.put | ||
| get = inqueue.get | ||
| if hasattr(inqueue, '_writer'): | ||
|
|
@@ -254,8 +256,8 @@ def _setup_queues(self): | |
| def apply(self, func, args=(), kwds={}): | ||
| ''' | ||
| Equivalent of `func(*args, **kwds)`. | ||
| Pool must be running. | ||
| ''' | ||
| assert self._state == RUN | ||
| return self.apply_async(func, args, kwds).get() | ||
|
|
||
| def map(self, func, iterable, chunksize=None): | ||
|
|
@@ -307,6 +309,10 @@ def imap(self, func, iterable, chunksize=1): | |
| )) | ||
| return result | ||
| else: | ||
| if chunksize < 1: | ||
| raise ValueError( | ||
| "Chunksize must be 1+, not {0:n}".format( | ||
| chunksize)) | ||
| assert chunksize > 1 | ||
| task_batches = Pool._get_tasks(func, iterable, chunksize) | ||
| result = IMapIterator(self._cache) | ||
|
|
@@ -334,7 +340,9 @@ def imap_unordered(self, func, iterable, chunksize=1): | |
| )) | ||
| return result | ||
| else: | ||
| assert chunksize > 1 | ||
| if chunksize < 1: | ||
| raise ValueError( | ||
| "Chunksize must be 1+, not {0!r}".format(chunksize)) | ||
| task_batches = Pool._get_tasks(func, iterable, chunksize) | ||
| result = IMapUnorderedIterator(self._cache) | ||
| self._taskqueue.put( | ||
|
|
@@ -466,7 +474,7 @@ def _handle_results(outqueue, get, cache): | |
| return | ||
|
|
||
| if thread._state: | ||
| assert thread._state == TERMINATE | ||
| assert thread._state == TERMINATE, "Thread not in TERMINATE" | ||
| util.debug('result handler found thread._state=TERMINATE') | ||
| break | ||
|
|
||
|
|
@@ -542,7 +550,10 @@ def terminate(self): | |
|
|
||
| def join(self): | ||
| util.debug('joining pool') | ||
| assert self._state in (CLOSE, TERMINATE) | ||
| if self._state == RUN: | ||
| raise ValueError("Pool is still running") | ||
| elif self._state not in (CLOSE, TERMINATE): | ||
| raise ValueError("In unknown state") | ||
| self._worker_handler.join() | ||
| self._task_handler.join() | ||
| self._result_handler.join() | ||
|
|
@@ -570,7 +581,9 @@ def _terminate_pool(cls, taskqueue, inqueue, outqueue, pool, | |
| util.debug('helping task handler/workers to finish') | ||
| cls._help_stuff_finish(inqueue, task_handler, len(pool)) | ||
|
|
||
| assert result_handler.is_alive() or len(cache) == 0 | ||
| if (not result_handler.is_alive()) and (len(cache) != 0): | ||
| raise AssertionError( | ||
| "Cannot have cache with result_hander not alive") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I get the point of adding a message here. The message will not be understandable for the average user, and whoever wants to debug it has to read the source code anyway.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does one tell where the AssertionError is happening, if the pool is being used not by the primary process but by a subprocess (possibly on another machine via a manager)?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks to the line number in the traceback, I guess?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wish that worked in the circumstances I noted above - see here, noting that it's a "nightly" build so includes the earlier patch, for an example. |
||
|
|
||
| result_handler._state = TERMINATE | ||
| outqueue.put(None) # sentinel | ||
|
|
@@ -628,7 +641,8 @@ def ready(self): | |
| return self._event.is_set() | ||
|
|
||
| def successful(self): | ||
| assert self.ready() | ||
| if not self.ready(): | ||
| raise ValueError("{0!r} not ready".format(self)) | ||
| return self._success | ||
|
|
||
| def wait(self, timeout=None): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this one is necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will get back to you on this one - need to put together lecture for tomorrow...
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would it not be necessary? Also see below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because otherwise would probably a bug in the underlying
recvmsgimplementation, which I think doesn't need to be guarded by a nice error message.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm actually more worried about detecting what bug is happening - see here for why, noting that it's with a nightly build thus has the first patch in it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, fair enough.