[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Change by Ned Deily : -- versions: -Python 2.7, Python 3.7 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: h

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Antoine Pitrou added the comment: New changeset 351c67416ba4451eb3928fa0b2e933c2f25df1a3 by Antoine Pitrou (Jeroen Demeyer) in branch 'master': bpo-35983: skip trashcan for subclasses (GH-11841) https://github.com/python/cpython/commit/351c67416ba4451eb3928fa0b2e933c2f25df1a3 -- __

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Change by Jeroen Demeyer : -- pull_requests: +12653 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://m

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Jeroen Demeyer added the comment: In Python 3, the resurrection issue probably appears too. But it's not so much a problem since __del__ (mapped to tp_finalize) is only called once anyway. So there are no bad consequences if the object is resurrected incorrectly. --

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Jeroen Demeyer added the comment: I realized that there is a nasty interaction between the trashcan and __del__: if you're very close to the trashcan limit and you're calling __del__, then objects that should have been deallocated in __del__ (in particular, an object involving self) might in

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Change by Jeroen Demeyer : -- pull_requests: +12624 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://m

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Jeroen Demeyer added the comment: I created an additional PR 12607 with some more changes to the, in particular to make the old backwards-compatibility trashcan macros safer. This should be seen as part of the same bugfix but I decided to make a new PR because PR 11841 had several positive r

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Change by Jeroen Demeyer : -- pull_requests: +12546 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://m

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Jeroen Demeyer added the comment: > It disables the trashcan mechanism Yes, it disables the trashcan in some cases. But only when using the trashcan mechanism would probably crash CPython anyway due to a double deallocation. So at the very least, PR 11841 improves things from "crashing whene

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Serhiy Storchaka added the comment: Yes, I think that there's something wrong with PR 11841. It disables the trashcan mechanism and does not provide other mechanism for solving that problem. -- ___ Python tracker

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Jeroen Demeyer added the comment: Changing types like that looks like an ugly hack and a recipe for breakage. For example, in list_dealloc(), the following needs the type to be correct: if (numfree < PyList_MAXFREELIST && PyList_CheckExact(op)) free_list[numfree++] = op; else

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Serhiy Storchaka added the comment: In your example you can add PyTypeObject *tp = Py_TYPE(self); Py_TYPE(self) = &PyList_Type; if (PyType_GetFlags(tp) & Py_TPFLAGS_HEAPTYPE) { Py_DECREF(tp); } before calling PyList_Type.tp_dealloc(). It is possible to add such code d

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Jeroen Demeyer added the comment: To clarify: the purpose of MyList is specifically to check that no double deallocations occur. For this test to make sense, MyList should not use the trashcan itself. -- ___ Python tracker

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Jeroen Demeyer added the comment: Yes of course. When not using the trashcan, stuff crashes. I don't get your point... -- ___ Python tracker ___ _

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Serhiy Storchaka added the comment: Disabling the trashcan mechanism returns the problem for solving which the trashcan mechanism was introduced. >>> from _testcapi import MyList >>> L = None >>> for i in range(100): ... L = MyList((L,)) ... >>> del L Segmentation fault (core dumped)

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Change by Serhiy Storchaka : -- nosy: +serhiy.storchaka ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Jeroen Demeyer added the comment: > Jeroen, could you share your example? I am learning the C-API of Python and > this example could be interesting. Either use the Cython code I posted above or run the testcase I added in PR 11841. -- ___ Python

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Jeroen Demeyer added the comment: NOTE: also OrderedDict currently uses trashcan hacking to work around this problem: /* Call the base tp_dealloc(). Since it too uses the trashcan mechanism, * temporarily decrement trash_delete_nesting to prevent triggering it * and putting the

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Eric Snow added the comment: On Wed, Feb 13, 2019 at 8:42 AM Antoine Pitrou wrote: > Antoine Pitrou added the comment: > > By "relatively new thing", you mean less than 20 years old? :-) Yeah, looks like it was in the 2.2 release (Dec 2001) for PEP 253. Anyway, I know of several core devs wh

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Change by Jeroen Demeyer : -- keywords: +patch pull_requests: +11871 stage: test needed -> patch review ___ Python tracker ___ ___ P

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Antoine Pitrou added the comment: By "relatively new thing", you mean less than 20 years old? :-) -- ___ Python tracker ___ ___ Pyt

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Eric Snow added the comment: FWIW, subclassing builtin types is a relatively new thing. There are still a number of lingering (older) implementation details throughout CPython that were written assuming no subclassing. I'd guess that this is one of them. -- nosy: +eric.snow stage:

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Jeroen Demeyer added the comment: The problem is easily reproduced with Cython: cdef class List(list): cdef int deallocated def __dealloc__(self): if self.deallocated: print("Deallocated twice!") self.deallocated = 1 L = None for i in range(10*

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

Stéphane Wirtel added the comment: Jeroen, could you share your example? I am learning the C-API of Python and this example could be interesting. -- nosy: +matrixise ___ Python tracker __

[issue35983] tp_dealloc trashcan shouldn't be called for subclasses

New submission from Jeroen Demeyer : When designing an extension type subclassing an existing type, it makes sense to call the tp_dealloc of the base class from the tp_dealloc of the subclass. Now suppose that I'm subclassing "list" which uses the trashcan mechanism. Then it can happen that t