invalid assert on big output of multiprocessing.Process

[ahcub]

BPO	34563
Nosy	@pitrou, @vstinner, @applio, @ahcub
PRs	bpo-34563 Fix for invalid assert on big output of multiprocessing.Process #9025 bpo-34563 Fix for invalid assert on big output of multiprocessing.Process #9026 bpo-34563: Fix for invalid assert on big output of multiprocessing.Process #9027 [3.7] bpo-34563: Fix for invalid assert on big output of multiprocessing.Process (GH-9027) #9064 [3.6] bpo-34563: Fix for invalid assert on big output of multiprocessing.Process (GH-9027) #9069
Files	invalid_assert_bug_reproduce.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-09-07.12:23:22.942>
created_at = <Date 2018-09-01.16:52:43.549>
labels = ['3.8', '3.7', 'library', 'type-crash']
title = 'invalid assert on big output of multiprocessing.Process'
updated_at = <Date 2018-09-07.15:37:51.831>
user = 'https://github.com/ahcub'

bugs.python.org fields:

activity = <Date 2018-09-07.15:37:51.831>
actor = 'ahcub'
assignee = 'none'
closed = True
closed_date = <Date 2018-09-07.12:23:22.942>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2018-09-01.16:52:43.549>
creator = 'ahcub'
dependencies = []
files = ['47778']
hgrepos = []
issue_num = 34563
keywords = ['patch']
message_count = 7.0
messages = ['324465', '324497', '324594', '324601', '324735', '324736', '324764']
nosy_count = 4.0
nosy_names = ['pitrou', 'vstinner', 'davin', 'ahcub']
pr_nums = ['9025', '9026', '9027', '9064', '9069']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue34563'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[ahcub]

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.

[ahcub]

By bigger than signed int i meant bigger than positive signed int

[vstinner]

New changeset 266f490 by Victor Stinner (Alexander Buchkovsky) in branch 'master':
bpo-34563: Fix for invalid assert on big output of multiprocessing.Process (GH-9027)
266f490

[vstinner]

New changeset e8ca880 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)
e8ca880

[vstinner]

New changeset 23f427a 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)
23f427a

[vstinner]

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.

[ahcub]

thank you for all the help in merging this fix!