classification
Title: Assigning and deleting __new__ attr on the class does not allow to create instances of this class
Type: behavior Stage:
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Carl Dunham, Stanisław Skonieczny (Uosiu), benjamin.peterson, bkabrda, eryksun, jcristau, python-dev, twouters
Priority: normal Keywords:

Created on 2015-11-25 12:29 by Stanisław Skonieczny (Uosiu), last changed 2016-11-18 16:16 by Carl Dunham.

Files
File name Uploaded Description Edit
new_patch_fails.py Stanisław Skonieczny (Uosiu), 2015-11-25 12:29 Attached file works on python 2.7, but fails on python3.5 with `TypeError: object() takes no parameters`.
Messages (10)
msg255337 - (view) Author: Stanisław Skonieczny (Uosiu) (Stanisław Skonieczny (Uosiu)) Date: 2015-11-25 12:29
When moving from python 2.7 to 3.5 I have found a problem with patching __new__ method on the class. It was done this way:
'''
patch('foo.bar.MyClass.__new__', return_value=mocked_instance)
'''
In works with python 2.7, but in 3.5 it fails with:
'''
TypeError: object() takes no parameters
'''

I have created minimal scenario that ilustrates this bug cause:
'''
class X:
    def __init__(self, a):
        pass


def new(cls, a):
    pass


X(1)
X.__new__ = new
X(1)
del X.__new__
X(1)
'''
Setting __new__ attribute and then deleting it has some side effect.
msg255371 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2015-11-25 17:47
For "del X.__new__", type_setattro in Objects/typeobject.c indirectly calls update_one_slot. This finds object.__new__ fom the base object class when it looks up __new__ on the type. Since __new__ for built-in types is special-cased to be a built-in method instead of a slot wrapper, update_one_slot takes the branch for "Py_TYPE(descr) == &PyCFunction_Type", which assigns the existing tp_new to "specific". In this case that's slot_tp_new instead of object_new due to the previous assignment of the "new" function to X.__new__.  

slot_tp_new looks up and calls __new__, which in this case, as noted above, is object.__new__. This built-in method calls tp_new_wrapper, which calls the wrapped tp_new function. In this case that's object_new. If the type's tp_init is object_init (i.e. not overridden) or tp_new is not object_new (i.e. overridden), then object_new raises a TypeError when called with arguments. The problem in this case is that __new__ isn't overridden anymore. 

It seems to me that update_one_slot needs to install the tp_new that tp_new_wrapper would call, e.g. 

    specific = (void *)(
                 (PyTypeObject *)PyCFunction_GET_SELF(descr))->tp_new;

In this case that's object_new. Thus after "del X.__new__", X would be restored as if __new__ was never overridden.
msg258530 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2016-01-18 17:11
Hi, I'm maintainer of flexmock [1] and some users of my library have started hitting this bug [2] - when one tries to mock __new__ and then revert this mock at the end of testcase to the original one (either by deleting the mock __new__ or replacing mock __new__ by the old one), then one gets "TypeError: object() takes no parameters".

I think that solution proposed by Eryk is correct, but I don't have that much insight into Python internals to confirm that 100 %. I would appreciate this getting fixed. Thanks!

[1] https://github.com/bkabrda/flexmock
[2] https://github.com/bkabrda/flexmock/issues/13
msg258572 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-01-19 05:18
New changeset 3c9512d8ac0d by Benjamin Peterson in branch '3.5':
set tp_new from the class in the hierarchy that actually owns the descriptor (closes #25731)
https://hg.python.org/cpython/rev/3c9512d8ac0d

New changeset e7062dd9085e by Benjamin Peterson in branch '2.7':
set tp_new from the class in the hierarchy that actually owns the descriptor (closes #25731)
https://hg.python.org/cpython/rev/e7062dd9085e

New changeset a7953ee29f1c by Benjamin Peterson in branch 'default':
merge 3.5 (#25731)
https://hg.python.org/cpython/rev/a7953ee29f1c
msg262922 - (view) Author: (jcristau) Date: 2016-04-05 20:56
This change in 2.7 seems to break things:

$ cat foo.pxd 
cdef class B:
    cdef object b
$ cat foo.pyx 
cdef class A:
    pass

cdef class B:
    def __init__(self, b):
        self.b = b
$ cat bar.py
from foo import A, B

class C(A, B):
    def __init__(self):
        B.__init__(self, 1)

C()
$ cython foo.pyx && gcc -I/usr/include/python2.7 -Wall -shared -fPIC -o foo.so foo.c
$ python -c 'import bar'
Segmentation fault

C's tp_new is set to A's tp_new function, thus the b slot is never initialized to Py_None, and C's __init__ calls DECREF on a NULL pointer.

Reverting changeset e7062dd9085e makes things work again, with C's tp_new being B's tp_new.
msg263580 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2016-04-16 19:17
I believe the correct behavior is actually

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "bar.py", line 7, in <module>
    C()
TypeError: foo.A.__new__(C) is not safe, use foo.B.__new__()

This is because A comes before B in the mro, and, indeed, constructing C with A.__new__ is unsafe. In fact, reordering A and B in the definition of C fixes everything.
msg263816 - (view) Author: (jcristau) Date: 2016-04-20 10:01
Well yes, but as far as I can tell that's why python used B.__new__ for C before your change, as that has a compatible layout.
msg266574 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2016-05-28 21:06
I'll have to think about how to fix this while maintaining compatiblity with obscure cases like above.
msg266576 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-05-28 21:08
New changeset 3ff84a3eeb6b by Benjamin Peterson in branch '2.7':
Backed out changeset e7062dd9085e (#25731)
https://hg.python.org/cpython/rev/3ff84a3eeb6b
msg278962 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2016-10-19 07:15
Hi all, it seems to me that this change has been reverted not only in 2.7, but also in 3.5 (changeset: 101549:c8df1877d1bc). Benjamin, was this intentional? If so, perhaps this issue should be reopened and not marked as resolved.

Thanks a lot!
History
Date User Action Args
2018-11-04 15:01:01serhiy.storchakalinkissue35162 superseder
2018-10-30 12:41:27josh.rlinkissue35098 superseder
2016-11-18 16:16:55Carl Dunhamsetnosy: + Carl Dunham
2016-10-19 08:03:21serhiy.storchakasetstage: resolved ->
resolution: fixed ->
versions: + Python 3.7, - Python 3.4
2016-10-19 07:15:17bkabrdasetmessages: + msg278962
2016-05-28 21:08:17python-devsetmessages: + msg266576
2016-05-28 21:06:11benjamin.petersonsetmessages: + msg266574
2016-04-20 10:01:48jcristausetmessages: + msg263816
2016-04-16 19:17:39benjamin.petersonsetmessages: + msg263580
2016-04-11 13:29:10barrysetstatus: closed -> open
2016-04-05 20:56:35jcristausetnosy: + jcristau
messages: + msg262922
2016-01-19 05:18:45python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg258572

resolution: fixed
stage: resolved
2016-01-18 17:11:09bkabrdasetnosy: + bkabrda
messages: + msg258530
2015-12-01 20:36:10eryksunsetnosy: + twouters, benjamin.peterson
2015-11-25 17:47:18eryksunsetnosy: + eryksun

messages: + msg255371
versions: + Python 2.7, Python 3.4, Python 3.6
2015-11-25 12:29:50Stanisław Skonieczny (Uosiu)create