Skip to content
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

Error when unpickling IncompleteReadError #76215

Closed
vwvw mannequin opened this issue Nov 15, 2017 · 8 comments
Closed

Error when unpickling IncompleteReadError #76215

vwvw mannequin opened this issue Nov 15, 2017 · 8 comments
Labels

Comments

@vwvw
Copy link
Mannequin

vwvw mannequin commented Nov 15, 2017

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
    Copy link
    Mannequin Author

    vwvw mannequin commented Nov 15, 2017

    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.

    @vwvw vwvw mannequin added the topic-asyncio label Nov 15, 2017
    @vwvw vwvw mannequin changed the title Impossible to unpickle IncompleteReadError Error when unpickling IncompleteReadError Nov 15, 2017
    @vwvw vwvw mannequin added the 3.7 (EOL) end of life label Nov 15, 2017
    @serhiy-storchaka
    Copy link
    Member

    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
    Copy link
    Mannequin Author

    vwvw mannequin commented Nov 15, 2017

    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
    Copy link
    Mannequin Author

    vwvw mannequin commented Nov 15, 2017

    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
    Copy link
    Member

    1st1 commented Nov 15, 2017

    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
    Copy link
    Member

    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
    Copy link
    Member

    1st1 commented Nov 15, 2017

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

    @1st1 1st1 closed this as completed Nov 15, 2017
    @1st1
    Copy link
    Member

    1st1 commented Nov 16, 2017

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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants