Skip to content

Handle wrapper specific fields never having been set in the C extension - #354

Merged
GrahamDumpleton merged 3 commits into
developfrom
bugfix/uninitialised-wrapper-call
Sep 20, 2026
Merged

GrahamDumpleton merged 3 commits into
developfrom
bugfix/uninitialised-wrapper-call

Conversation

@GrahamDumpleton

Copy link
Copy Markdown
Owner

Supersedes the second part of #351. Companion to #353, which handles the
first part.

Problem

Calling a FunctionWrapper, BoundFunctionWrapper or
PartialCallableObjectProxy for which __init__() was never called, but
which had __wrapped__ assigned directly, crashed the interpreter when
the C extension was in use. The realistic way to get there is a derived
class which overrides __init__() and sets __wrapped__ itself without
calling __init__() of the base class:

class Wrapper(wrapt.FunctionWrapper):
    def __init__(self, wrapped):
        self.__wrapped__ = wrapped

Wrapper(function)()   # SIGSEGV

An instance created using __new__() alone behaves the same. The existing
check for an uninitialized wrapper only considers whether __wrapped__ is
set, so it passed, and the additional fields of the wrapper (instance,
wrapper, enabled, binding, parent, owner, or args for a
partial) were then used while still NULL.

Four code paths were affected:

path before
FunctionWrapper.__call__ crash
BoundFunctionWrapper.__call__ crash
PartialCallableObjectProxy.__call__ crash
FunctionWrapper.__get__ returned a wrapper built from the NULL fields, which crashed when called

The pure Python implementation raised AttributeError in these cases, as
it holds the same state in instance attributes which do not exist. The
exception was BoundFunctionWrapper, which failed with RecursionError,
including on merely assigning __wrapped__, because its __getattr__()
looked up _self_parent on itself and recursed when it was not set.

Separately, reading the same fields through the _self_ attributes did
not crash, but the C extension substituted None, or an empty tuple or
dictionary for a partial, where the pure Python implementation raised
AttributeError. None is a valid value for _self_instance,
_self_enabled, _self_parent and _self_owner, so the C extension
reported a plausible state for a wrapper which had none.

Changes

  • C extension: a new wrapt_require_field() helper, applied in each of
    the four functions above to the fields once they have been acquired.
    The check is made on the strong references already held, so adds no
    further unlocked reads on a free-threaded build. One Py_DECREF in the
    cleanup of the partial became Py_XDECREF so that the error path is
    safe.
  • C extension: the getters for the _self_ attributes use the same
    helper in place of substituting a default. As the lookup of a missing
    attribute on a proxy falls through to __getattr__() and on to the
    wrapped object, the result is exactly that of the pure Python
    implementation, including the attribute of the wrapped object being
    returned where that is itself a wrapper.
  • The captured keyword arguments of a partial are legitimately NULL when
    none were supplied, so they are not required when calling it, and
    _self_kwargs continues to be an empty dictionary in that case. It
    fails only where the captured positional arguments, which __init__()
    always sets, are NULL as well.
  • Pure Python: BoundFunctionWrapper.__getattr__() raises
    AttributeError for _self_parent itself rather than recursing.
  • Tests: calls, descriptor access and each _self_ attribute, for
    wrappers created both by __new__() alone and by a derived class not
    calling the base __init__(), across all three types; that the error
    for an attribute is the one the wrapped object gives; that a wrapped
    wrapper's attributes are returned; and that initialized wrappers,
    including partials with and without keyword arguments, are unaffected.
  • Changelog entry under 2.4.2.

Choice of exception

AttributeError naming the missing _self_ attribute, rather than
WrapperNotInitializedError. It is what the pure Python implementation
already raises, so the two agree without adding a try/except to the pure
Python call path, and it follows the existing convention in
raise_uninitialized_wrapper_error(), where __init__() never having
been called is an AttributeError, and WrapperNotInitializedError is
kept for __init__() having run but __wrapped__ since being removed.
The message has the same form as the existing one:

'_wrappers.FunctionWrapper' object has no attribute '_self_instance'

Compatibility

The change to the _self_ attributes alters behaviour which did not
crash. Code which reads one of them, or tests for it with hasattr(),
before __init__() of the base class has been called would have worked
with the C extension and will now raise. Such code already failed with
the pure Python implementation, so was not portable.

Relationship to #351

A crash on calling an uninitialized FunctionWrapper was first noted by
Ding Qiuran in #351, found by static analysis, which added a guard for
the enabled field in two of the functions. The other fields acquired
alongside it were still used unchecked, so the crash became a
SystemError rather than being resolved, and the partial and descriptor
paths were not covered. This PR addresses all of the fields and all of
the affected paths.

Note on merging

This and #353 both add their changelog entry directly under
**Bugs Fixed** for 2.4.2, so whichever is merged second will have a
trivial conflict in docs/changes.rst.

Verification

just test passes across the full matrix (3.9 to 3.15, three variants
each).

Calling a FunctionWrapper, BoundFunctionWrapper or
PartialCallableObjectProxy for which __init__() was never called, but
which had __wrapped__ assigned directly, crashed the interpreter with the
C extension. Accessing such a FunctionWrapper as a descriptor did not
crash, but returned a wrapper built from the unset fields which crashed
when called. The check for an uninitialized wrapper only considers
__wrapped__, so it passed and the additional fields were then used while
still NULL. This arises where a derived class overrides __init__() and
sets __wrapped__ itself without calling __init__() of the base class, or
an instance is created using __new__() alone.

The fields are now checked once acquired and an AttributeError naming the
missing _self_ attribute is raised, as the pure Python implementation
already does. The pure Python BoundFunctionWrapper instead failed with a
RecursionError, as its __getattr__() looked up _self_parent on itself, and
now raises AttributeError too.

A crash on calling an uninitialized FunctionWrapper was first noted by
Ding Qiuran in #351, which guarded one of the fields involved. This
addresses all of the fields and the other affected code paths.
…tes.

When __init__() had never been called, the getters in the C extension for
the _self_ attributes of a function wrapper returned None, and those of a
PartialCallableObjectProxy an empty tuple or dictionary, where the pure
Python implementation raised AttributeError as the instance attributes do
not exist. None is a valid value for most of these attributes, so the
result was indistinguishable from that of an initialized wrapper.

The getters now fail with AttributeError. The attribute lookup then falls
through to __getattr__() and on to the wrapped object, so the outcome is
the same as for the pure Python implementation, including the attribute
of the wrapped object being returned where that is itself a wrapper.

The captured keyword arguments of a partial are legitimately unset when
none were supplied, so _self_kwargs only fails where the captured
positional arguments, which __init__() always sets, are unset as well.

The tests are moved to test_uninitialized_wrapper.py as they no longer
only cover calling a wrapper.
Brings in the fix for deletion of __module__ and __doc__ on a proxy from
#353. Both changes added their release note at the same position for
version 2.4.2 in docs/changes.rst. Both are kept, with that for #353 first
as it was merged first.
@GrahamDumpleton
GrahamDumpleton merged commit 55410ca into develop Sep 20, 2026
49 checks passed
@GrahamDumpleton
GrahamDumpleton deleted the bugfix/uninitialised-wrapper-call branch September 20, 2026 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant