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

Exception copy error #87626

Open
douglas-raillard-arm mannequin opened this issue Mar 10, 2021 · 3 comments
Open

Exception copy error #87626

douglas-raillard-arm mannequin opened this issue Mar 10, 2021 · 3 comments
Labels
3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@douglas-raillard-arm
Copy link
Mannequin

douglas-raillard-arm mannequin commented Mar 10, 2021

BPO 43460
Nosy @iritkatriel, @douglas-raillard-arm

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 = None
created_at = <Date 2021-03-10.11:30:36.736>
labels = ['type-bug', 'library', '3.11']
title = 'Exception copy  error'
updated_at = <Date 2021-06-27.21:31:34.424>
user = 'https://github.com/douglas-raillard-arm'

bugs.python.org fields:

activity = <Date 2021-06-27.21:31:34.424>
actor = 'iritkatriel'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-03-10.11:30:36.736>
creator = 'douglas-raillard-arm'
dependencies = []
files = []
hgrepos = []
issue_num = 43460
keywords = []
message_count = 3.0
messages = ['388427', '388428', '396600']
nosy_count = 2.0
nosy_names = ['iritkatriel', 'douglas-raillard-arm']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue43460'
versions = ['Python 3.11']

@douglas-raillard-arm
Copy link
Mannequin Author

douglas-raillard-arm mannequin commented Mar 10, 2021

Instances of subclasses of BaseException created with keyword argument fail to copy properly as demonstrated by:

    import copy

    class E(BaseException):
        def __init__(self, x):
            self.x=x

    # works fine
    e = E(None)
    copy.copy(e)

    # raises
    e = E(x=None)
    copy.copy(e)

This seems to affect all Python versions I've tested (3.6 <= Python <= 3.9).

I've currently partially worked around the issue with a custom pickler that just restores __dict__, but:

  • "args" is not part of __dict__, and setting "args" key in __dict__ does not create a "working object" (i.e. the key is set, but is ignored for all intents and purposes except direct lookup in __dict__)

  • pickle is friendly: you can provide a custom pickler that chooses the reduce function for each single class.
    copy module is much less friendly: copyreg.pickle() only allow registering custom functions for specific classes. That means there is no way (that I know) to make copy.copy() select a custom reduce for a whole subclass tree.

One the root of the issue:

  • exception from the standard library prevent keyword arguments (maybe because of that issue ?), but there is no such restriction on user-defined classes.
  • the culprit is BaseException_reduce() (in Objects/exceptions.c) [1]

It seems that the current behavior is a consequence of the __dict__ being created lazily, I assume for speed and memory efficiency

There seems to be a few approaches that would solve the issue:

  • keyword arguments passed to the constructor could be fused with the positional arguments in BaseException_new (using the signature, but signature might be not be available for extension types I suppose)

  • keyword arguments could just be stored like "args" in a "kwargs" attribute in PyException_HEAD, so they are preserved and passed again to __new__ when the instance is restored upon copying/pickling.

  • the fact that keyword arguments were used could be saved as a bool in PyException_HEAD. When set, this flag would make BaseException_reduce() only use __dict__ and not "args". This would technically probably be a breaking change, but the only cases I can think of where this would be observable are a bit far fetched (if __new__ or __init__ have side effects beyond storing attributes in __dict__).

[1] https://github.com/python/cpython/blob/master/Objects/exceptions.c#L134

@douglas-raillard-arm douglas-raillard-arm mannequin added type-bug An unexpected behavior, bug, or error labels Mar 10, 2021
@douglas-raillard-arm
Copy link
Mannequin Author

douglas-raillard-arm mannequin commented Mar 10, 2021

The solution based on the signature is something along those lines:

    class E(BaseException):
        def __new__(cls, *args, **kwargs):
            """
            Fix exception copying.
        Turn all the keyword arguments into positional arguments, so that the
        :exc:`BaseException` machinery has all the parameters for a valid call
        to ``__new__``, instead of missing all the keyword arguments.
        """
        sig = inspect.signature(cls.__init__)
        bound_args = sig.bind_partial(*args, **kwargs)
        bound_args.apply_defaults()
        args = tuple(bound_args.arguments.values())
        return super().__new__(cls, *args)
        def __init__(self, x):
            self.x=x

But there are a many shortcomings to that approach:

  • What if super().__new__() consumes arguments before passing the rest to __init__() ? This fix is blind to that since it only cares about __init__ signature

  • What if inspect.signature() does not provide a signature (extension modules) ?

  • Defaults are "hardcoded" in the args, so the object will always be restored with the defaults of the time it was created. This is a breaking change, as currently the defaults used when restoring the instance are the current ones.

  • Also uses more memory for args (and for pickle files), since it contains all the defaults

@iritkatriel
Copy link
Member

See bpo-32696, bpo-30005, bpo-29466

@iritkatriel iritkatriel added stdlib Python modules in the Lib dir 3.11 only security fixes labels Jun 27, 2021
@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
3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
Status: No status
Development

No branches or pull requests

1 participant