Issue 17482: functools.update_wrapper mishandles wrapped (original) (raw)
Created on 2013-03-19 17:35 by ncoghlan, last changed 2022-04-11 14:57 by admin. This issue is now closed.
Messages (12)
Author: Alyssa Coghlan (ncoghlan) * 
Date: 2013-03-19 17:35
functools.update_wrapper inadvertently overwrites the just set wrapped attribute when it updates the contents of dict.
This means the intended wrapped chain is never created - instead, for every function in a wrapper stack, wrapped will always refer to the innermost function.
This means that using wrapped to bypass functools.lru_cache doesn't work correctly if the decorated function already has wrapped set.
Explicitly setting signature fortunately still works correctly, since that is checked before recursing down through wrapped in inspect.signature.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2013-03-19 17:44
There's an interesting backwards compatibility challenge here. We definitely need to fix the misbehaviour, since it can lead to some pretty serious bugs in user code when attempting to bypass the LRU cache decorator if the wrapped function itself had a wrapped attribute.
However, Michael tells me that at least some third party clients of the introspection tools assumed the "wrapped points to the bottom of the wrapper stack" behaviour was intentional rather than just me screwing up the implementation.
The existing docs for update_wrapper are unfortunately ambiguous because they use the term "original function" instead of "wrapped function".
Author: Alyssa Coghlan (ncoghlan) *
Date: 2013-03-19 18:17
And as further evidence that I always intended this to be a wrapper chain: issue 13266 :)
Author: Alyssa Coghlan (ncoghlan) *
Date: 2013-03-19 18:25
OK, thinking about this a little further, I think it's not as bad as I feared. The number of people likely to be introspecting wrapped is quite small, and updating to the correct recursive code will still do the right thing in existing versions.
So long as we tell people this is coming, and get Georg to highlight it in the release notes for the corresponding 3.3.x point release, we should be able to fix the lru_cache bug without too much collateral damage.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2013-05-24 13:33
Georg, just a heads up that I was informed of a fairly significant bug in the wrapped handling which some folks doing function introspection had been assuming was a feature.
I'd like to fix it for 3.3.3, but need your +1 as RM since it will break introspection code which assumes the current behaviour was intentional.
Author: Roundup Robot (python-dev) 
Date: 2013-07-15 11:13
New changeset 13b8fd71db46 by Nick Coghlan in branch 'default': Close issue 17482: don't overwrite wrapped http://hg.python.org/cpython/rev/13b8fd71db46
Author: Alyssa Coghlan (ncoghlan) *
Date: 2013-07-15 11:19
I decided I can live with the risk of this biting someone in 3.3 - the combination of using multiple levels of wrapping and using wrapped for more than merely introspection seems remote enough to make being conservative with the behavioural change the better course.
Author: mike bayer (zzzeek) *
Date: 2014-02-19 00:58
this is actually biting me, I think, though I'm having a very hard time getting a small Python script to duplicate it :/. https://bitbucket.org/zzzeek/alembic/issue/175/test-suite-failure-under-python34 refers to the current problems I'm having. I am not really familiar with the ramifications of wrapped so I need to learn some more about that but also I'm concerned that a standalone test which attempts to simulate everything as much as possible is not doing the same thing.
Author: mike bayer (zzzeek) *
Date: 2014-02-19 01:15
i think I found the problem. sorry for the noise.
Author: mike bayer (zzzeek) *
Date: 2014-02-19 01:38
OK well, let me just note what the issue is, and I think this is pretty backwards-incompatible, and additionally I really can't find any reasonable way of working around it except for just deleting wrapped. It would be nice if there were some recipe or documentation that could point people to how do do the following pattern:
import functools import inspect
def my_wrapper(fn): def wrapped(x, y, z): return my_func(x, y) wrapped = functools.update_wrapper(wrapped, fn) return wrapped
def my_func(x, y): pass
wrapper = my_wrapper(my_func)
passes for 2.6 - 3.3, fails on 3.4
assert inspect.getargspec(wrapper) == (['x', 'y', 'z'], None, None, None), inspect.getargspec(wrapper)
basically in Alembic we copy out a bunch of decorated functions out somewhere else using inspect(), and that code relies upon seeing the wrappers list of arguments, not the wrapped. Not that Python 3.4's behavior isn't correct now, but this seems like something that might be somewhat common.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2014-02-19 05:26
Mike, could you file a new issue for that? It's a genuine regression in the inspect module.
Author: Alyssa Coghlan (ncoghlan) *
Date: 2014-02-19 12:40
New release blocker created as issue 20684
History
Date
User
Action
Args
2022-04-11 14:57:43
admin
set
github: 61684
2021-11-04 14:33:49
eryksun
set
messages: -
2021-11-04 14:33:37
eryksun
set
nosy: - ahmedsayeed1982
components: - IO
versions: - Python 3.9
2021-11-04 12:11:10
ahmedsayeed1982
set
versions: + Python 3.9, - Python 3.4
nosy: + ahmedsayeed1982, - georg.brandl, rhettinger, jcea, ncoghlan, ezio.melotti, zzzeek, michael.foord, lukasz.langa, python-dev
messages: +
components: + IO
2014-02-19 12:40:46
ncoghlan
set
messages: +
2014-02-19 05:26:02
ncoghlan
set
messages: +
2014-02-19 01:38:12
zzzeek
set
messages: +
2014-02-19 01:15:16
zzzeek
set
messages: +
2014-02-19 00:58:50
zzzeek
set
nosy: + zzzeek
messages: +
2013-07-15 11:19:16
ncoghlan
set
messages: +
versions: - Python 3.3
2013-07-15 11:13:30
python-dev
set
status: open -> closed
nosy: + python-dev
messages: +
resolution: fixed
stage: test needed -> resolved
2013-05-24 13:42:48
lukasz.langa
set
nosy: + lukasz.langa
2013-05-24 13:33:13
ncoghlan
set
nosy: + georg.brandl
messages: +
2013-05-24 13:27:13
ncoghlan
set
assignee: ncoghlan
2013-03-25 03:16:24
jcea
set
nosy: + jcea
2013-03-24 13:56:31
ezio.melotti
set
nosy: + ezio.melotti
2013-03-19 18:25:44
ncoghlan
set
messages: +
2013-03-19 18:17:51
ncoghlan
set
messages: +
2013-03-19 18:16:05
ncoghlan
link
2013-03-19 17:44:45
ncoghlan
set
nosy: + rhettinger, michael.foord
title: functools.update_wrapper doesn't overwrite attributes correctly -> functools.update_wrapper mishandles __wrapped__
messages: +
versions: + Python 3.3
2013-03-19 17:35:42
ncoghlan
create