classification
Title: invalid assert on big output of multiprocessing.Process
Type: crash Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ahcub, davin, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2018-09-01 16:52 by ahcub, last changed 2018-09-07 15:37 by ahcub. This issue is now closed.

Files
File name Uploaded Description Edit
invalid_assert_bug_reproduce.py ahcub, 2018-09-01 16:52
Pull Requests
URL Status Linked Edit
PR 9025 closed python-dev, 2018-09-01 16:53
PR 9026 closed ahcub, 2018-09-01 17:03
PR 9027 merged ahcub, 2018-09-01 17:15
PR 9064 merged miss-islington, 2018-09-04 16:11
PR 9069 merged ahcub, 2018-09-05 01:28
Messages (7)
msg324465 - (view) Author: Oleksandr Buchkovskyi (ahcub) * Date: 2018-09-01 16:52
the bug is reproduced on big multiprocessing.Process output
when the size of the output gets bigger than signed int the value becomes negative, thus
```            
assert left > 0
```
in multiprocessing/connection.py:337 raises an exception like the following

```
Traceback (most recent call last):
  File "D:\GitHub\cpython\lib\threading.py", line 917, in _bootstrap_inner
    self.run()
  File "D:\GitHub\cpython\lib\threading.py", line 865, in run
    self._target(*self._args, **self._kwargs)
  File "D:\GitHub\cpython\lib\multiprocessing\pool.py", line 470, in _handle_results
    task = get()
  File "D:\GitHub\cpython\lib\multiprocessing\connection.py", line 250, in recv
    buf = self._recv_bytes()
  File "D:\GitHub\cpython\lib\multiprocessing\connection.py", line 318, in _recv_bytes
    return self._get_more_data(ov, maxsize)
  File "D:\GitHub\cpython\lib\multiprocessing\connection.py", line 337, in _get_more_data
    assert left > 0
AssertionError
```


this assert looks invalid in this case because in C code the left value is DWORD (unsigned long), which cannot be negative by definition.
msg324497 - (view) Author: Oleksandr Buchkovskyi (ahcub) * Date: 2018-09-03 06:40
By bigger than signed int i meant bigger than positive signed int
msg324594 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-09-04 16:10
New changeset 266f4904a222a784080e29aad0916849e507515d by Victor Stinner (Alexander Buchkovsky) in branch 'master':
bpo-34563: Fix for invalid assert on big output of multiprocessing.Process (GH-9027)
https://github.com/python/cpython/commit/266f4904a222a784080e29aad0916849e507515d
msg324601 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-09-04 19:40
New changeset e8ca8806a9f47b3bac4ae1c6b8a54c47d1aad8f3 by Victor Stinner (Miss Islington (bot)) in branch '3.7':
bpo-34563: Fix for invalid assert on big output of multiprocessing.Process (GH-9027) (GH-9064)
https://github.com/python/cpython/commit/e8ca8806a9f47b3bac4ae1c6b8a54c47d1aad8f3
msg324735 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-09-07 12:17
New changeset 23f427a0fdb888212136cf8745a9f5f832a3f374 by Victor Stinner (Alexander Buchkovsky) in branch '3.6':
[3.6] bpo-34563: Fix for invalid assert on big output of multiprocessing.Process (GH-9027) (GH-9069)
https://github.com/python/cpython/commit/23f427a0fdb888212136cf8745a9f5f832a3f374
msg324736 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-09-07 12:23
Thanks Alexander Buchkovsky for the fix: it has been merged into 3.6, 3.7 and master branches. Thanks Oleksandr Buchkovskyi for the bug report!

--

I looked at the Python 2.7 code and it doesn't look to be impacted by this bug.

Modules/_multiprocessing/pipe_connection.c uses ReadFile() and PeekNamedPipe(), but the result type of the two C functions using it is not a PyObject* and conn_recv_string() looks good to me. Moreover, the type of the buflength parameter of conn_recv_string() is size_t, so there is no signed issue.
msg324764 - (view) Author: Oleksandr Buchkovskyi (ahcub) * Date: 2018-09-07 15:37
thank you for all the help in merging this fix!
History
Date User Action Args
2018-09-07 15:37:51ahcubsetmessages: + msg324764
2018-09-07 12:23:22vstinnersetstatus: open -> closed
versions: + Python 3.8
messages: + msg324736

resolution: fixed
stage: patch review -> resolved
2018-09-07 12:17:24vstinnersetmessages: + msg324735
2018-09-05 01:28:28ahcubsetpull_requests: + pull_request8528
2018-09-04 19:40:08vstinnersetmessages: + msg324601
2018-09-04 16:11:12miss-islingtonsetpull_requests: + pull_request8524
2018-09-04 16:10:37vstinnersetnosy: + vstinner
messages: + msg324594
2018-09-03 06:40:26ahcubsetmessages: + msg324497
2018-09-03 06:37:51ahcubsetversions: + Python 3.7
2018-09-03 04:20:41xiang.zhangsetnosy: + pitrou, davin
2018-09-01 17:15:13ahcubsetpull_requests: + pull_request8494
2018-09-01 17:03:21ahcubsetpull_requests: + pull_request8493
2018-09-01 16:53:52python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request8492
2018-09-01 16:52:43ahcubcreate