Error when unpickling IncompleteReadError

[vwvw]

BPO	32034
Nosy	@serhiy-storchaka, @1st1, @vwvw
PRs	bpo-32034 Fix possible unpickling error in asyncio.IncompleteReadError #4404 bpo-32034: Make asyncio.IncompleteReadError pickleable. #4409 [3.6] bpo-32034: Make IncompleteReadError & LimitOverrunError pickleable GH-4409 #4411
Files	Impossible_to_unpickle_IncompleteReadError.py: python script highlighting the issue

^{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 2017-11-15.22:15:15.157>
created_at = <Date 2017-11-15.12:53:23.296>
labels = ['3.7', 'expert-asyncio']
title = 'Error when unpickling IncompleteReadError'
updated_at = <Date 2017-11-16.00:28:28.464>
user = 'https://github.com/vwvw'

bugs.python.org fields:

activity = <Date 2017-11-16.00:28:28.464>
actor = 'yselivanov'
assignee = 'none'
closed = True
closed_date = <Date 2017-11-15.22:15:15.157>
closer = 'yselivanov'
components = ['asyncio']
creation = <Date 2017-11-15.12:53:23.296>
creator = 'badouxn'
dependencies = []
files = ['47266']
hgrepos = []
issue_num = 32034
keywords = ['patch']
message_count = 8.0
messages = ['306268', '306270', '306289', '306292', '306297', '306305', '306316', '306324']
nosy_count = 3.0
nosy_names = ['serhiy.storchaka', 'yselivanov', 'badouxn']
pr_nums = ['4404', '4409', '4411']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue32034'
versions = ['Python 3.7']

[vwvw]

When using asyncio in combination with multiprocessing, a TypeError occur when readuntil() encounter an EOF instead of the delimiter.
readuntil return a IncompleteReadError exception which is pickled by the multiprocessing package.
The unpickling failed due to an argument being None. As noted in the following links, it fails:
https://stackoverflow.com/questions/16244923/how-to-make-a-custom-exception-class-with-multiple-init-args-pickleable

The error trace is: 
Exception in thread Thread-3:
Traceback (most recent call last):
  File "/usr/lib64/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/usr/lib64/python3.6/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib64/python3.6/multiprocessing/pool.py", line 463, in _handle_results
    task = get()
  File "/usr/lib64/python3.6/multiprocessing/connection.py", line 252, in recv
    return _ForkingPickler.loads(r)
TypeError: __init__() missing 1 required positional argument: 'expected'

Fix proposed:
Make the "expected" parameter of the IncompleteReadError exception class optional.

[serhiy-storchaka]

This isn't so simple.

>>> from asyncio.streams import IncompleteReadError
>>> import pickle
>>> e1 = IncompleteReadError(b'abc', 10)
>>> e2 = pickle.loads(pickle.dumps(e1))
>>> print(e1)
3 bytes read on a total of 10 expected bytes
>>> print(e2)
44 bytes read on a total of None expected bytes

[vwvw]

My first fix was indeed wrong.
Upon looking further into it, it seems to me that the problem come from the fact that the call to super is not done with the right argument.
Upon unpickling, the argument that will be passed to the __init__ is the string representation built on line 33-34.
That's why, when leaving expected as optional the number returned is 44, the length of the string.

I went looking for similar Exception class in the code base and found the MaybeEncodingError in multiprocessing/pool.py

By replacing the Error content with this one I don't have any error anymore. The message is kept identical.
'''
def __init__(self, partial, expected):
super().__init__(partial, expected)
self.partial = partial
self.expected = expected
def __str__(self):
return ("%d bytes read on a total of %r expected bytes" % (len(partial), expected))
'''

Does such a fix looks correct to you ?

[vwvw]

Typo in the last comment. The code should be:

'''
def __init__(self, partial, expected):
super().__init__(partial, expected)
self.partial = partial
self.expected = expected
def __str__(self):
return ("%d bytes read on a total of %r expected bytes" % (len(self.partial), self.expected))
'''

[1st1]

Does such a fix looks correct to you ?

No, this fix won't restore exception.args properly. I made a PR with a fix.

[serhiy-storchaka]

There is the same issue with LimitOverrunError.

I'm not sure that we should keep args the same. It affects __str__ and __repr__. badouxn overrides __str__. And the result of __repr__ currently looks like the exception constructor, but actually is not compatible with it.

>>> IncompleteReadError(b'abc', 10)
IncompleteReadError('3 bytes read on a total of 10 expected bytes',)

[1st1]

New changeset 43605e6 by Yury Selivanov in branch 'master':
bpo-32034: Make IncompleteReadError & LimitOverrunError pickleable bpo-4409
43605e6

[1st1]

New changeset f35076a by Yury Selivanov (Miss Islington (bot)) in branch '3.6':
bpo-32034: Make IncompleteReadError & LimitOverrunError pickleable #48659 (bpo-4411)
f35076a