Hi everybody, I'd like to revive this thread as I feel like we have to do something here but some consensus is needed first. To recap, the current state of things is as follows: - in March 2000 (d724b23420f) Christian Tismer contributed the "trashcan" patch that added Py_TRASHCAN_SAFE_BEGIN and Py_TRASHCAN_SAFE_END macros which allow destroying nested objects non-recursively. - in May 2019 (GH-11841 of BPO-35983) Antoine Pitrou merged a change by Jeroen Demeyer which made Py_TRASHCAN_SAFE_BEGIN/END (unintentionally?) backwards incompatible; this was released in Python 3.8.0. - by the way, GH-11841 introduced a new pair of macros (because they have different signatures) called simply Py_TRASHCAN_BEGIN and Py_TRASHCAN_END. - by that time there was already a follow-up PR open (GH-12607) to improve backwards compatibility of the macros, as well as introduce tests for them; this was never merged. - in Feb 2020 (0fa4f43db08) Victor Stinner removed the trashcan mechanism from the limited C API (note: not ABI, those are macros) since it accesses fields of structs not exposed in the limited C API; this was released in Python 3.9.0. - in May 2020 Irit noticed that the backwards incompatibility (BPO-40608) causes segfaults for C API code that worked fine with Python 3.7. Using the new macros requires code changes but doesn't crash. Now, there are a couple of things we can do here: Option 1: Finish GH-12607 to fix the old macros, keeping in mind this will restore compatibility lost with Python 3.8 - 3.10 only for users of 3.11+ Option 2: Review and merge GH-20104 that reverts the macro changes that make old client code segfault -- unclear what else this needs and again, that would only fix it for users of 3.11+ Option 3: Abandon GH-12607 and GH-20104, instead declaring the old macros deprecated for 3.11 and remove them in 3.13 I personally agree with Irit, voting +1 for Option 3 since the old macros were soft-deprecated already by introducing new macros in 3.8, and more importantly made incompatible with pre-3.8 usage. Let's talk on how to proceed. - Ł

On 26 Apr 2021, at 23:55, Irit Katriel via Python-Dev wrote:

Re https://bugs.python.org/issue40608 https://bugs.python.org/issue40608.

I think it will be an act of kindness to deprecate Py_TRASHCAN_SAFE_BEGIN/END in 3.10 and tell people to use Py_TRASHCAN_BEGIN/END instead.

TL;DR: There was a change in 3.8 that introduced the latter while leaving the former for backwards compatibility, but also inadvertently breaking them. This is not an easy bug to deal with in the wild, we found it because we have a unit test in our codebase referencing https://bugs.python.org/issue16602 http://bugs.python.org/issue16602. A deprecation note pointing to the new macros would have made it easier.

Is there any reason not to deprecate the old macros?

Irit _______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/LWP6MOLP... Code of Conduct: http://python.org/psf/codeofconduct/

I don't know if it is pertinent to this at all, but I raised https://bugs.python.org/issue44449 in which the faulthandler module can lead to a segfault inside Py_TRASHCAN_SAFE_BEGIN. Would that be avoided if frameobject.c was changed to use Py_TRASHCAN_BEGIN / END? Duncan. On Tue, 2021-08-17 at 12:00 +0200, Łukasz Langa wrote:

Hi everybody, I'd like to revive this thread as I feel like we have to do something here but some consensus is needed first.

To recap, the current state of things is as follows: - in March 2000 (d724b23420f) Christian Tismer contributed the "trashcan" patch that added Py_TRASHCAN_SAFE_BEGIN and Py_TRASHCAN_SAFE_END macros which allow destroying nested objects non-recursively. - in May 2019 (GH-11841 of BPO-35983) Antoine Pitrou merged a change by Jeroen Demeyer which made Py_TRASHCAN_SAFE_BEGIN/END (unintentionally?) backwards incompatible; this was released in Python 3.8.0. - by the way, GH-11841 introduced a new pair of macros (because they have different signatures) called simply  Py_TRASHCAN_BEGIN and Py_TRASHCAN_END. - by that time there was already a follow-up PR open (GH-12607) to improve backwards compatibility of the macros, as well as introduce tests for them; this was never merged. - in Feb 2020 (0fa4f43db08) Victor Stinner removed the trashcan mechanism from the limited C API (note: not ABI, those are macros) since it accesses fields of structs not exposed in the limited C API; this was released in Python 3.9.0. - in May 2020 Irit noticed that the backwards incompatibility (BPO- 40608) causes segfaults for C API code that worked fine with Python 3.7. Using the new macros requires code changes but doesn't crash.

Now, there are a couple of things we can do here: Option 1: Finish GH-12607 to fix the old macros, keeping in mind this will restore compatibility lost with Python 3.8 - 3.10 only for users of 3.11+ Option 2: Review and merge GH-20104 that reverts the macro changes that make old client code segfault -- unclear what else this needs and again, that would only fix it for users of 3.11+ Option 3: Abandon GH-12607 and GH-20104, instead declaring the old macros deprecated for 3.11 and remove them in 3.13

I personally agree with Irit, voting +1 for Option 3 since the old macros were soft-deprecated already by introducing new macros in 3.8, and more importantly made incompatible with pre-3.8 usage.

Let's talk on how to proceed.

- Ł

-- Duncan Grisby

On 17. 08. 21 12:00, Łukasz Langa wrote:

Hi everybody, I'd like to revive this thread as I feel like we have to do something here but some consensus is needed first.

To recap, the current state of things is as follows: - *in March 2000* (d724b23420f) Christian Tismer contributed the "trashcan" patch that added Py_TRASHCAN_SAFE_BEGIN and Py_TRASHCAN_SAFE_END macros which allow destroying nested objects non-recursively. - *in May 2019* (GH-11841 of BPO-35983) Antoine Pitrou merged a change by Jeroen Demeyer which made Py_TRASHCAN_SAFE_BEGIN/END (unintentionally?) backwards incompatible; this was released in Python 3.8.0. - by the way, GH-11841 introduced a new pair of macros (because they have different signatures) called simply  Py_TRASHCAN_BEGIN and Py_TRASHCAN_END. - by that time there was already a follow-up PR open (GH-12607) to improve backwards compatibility of the macros, as well as introduce tests for them; this was never merged. - *in Feb 2020* (0fa4f43db08) Victor Stinner removed the trashcan mechanism from the limited C API (note: not ABI, those are macros) since it accesses fields of structs not exposed in the limited C API; this was released in Python 3.9.0. - *in May 2020* Irit noticed that the backwards incompatibility (BPO-40608) causes segfaults for C API code that worked fine with Python 3.7. Using the new macros requires code changes but doesn't crash.

Now, there are a couple of things we can do here: *Option 1*: Finish GH-12607 to fix the old macros, keeping in mind this will restore compatibility lost with Python 3.8 - 3.10 only for users of 3.11+ *Option 2*: Review and merge GH-20104 that reverts the macro changes that make old client code segfault -- unclear what else this needs and again, that would only fix it for users of 3.11+ *Option 3*: Abandon GH-12607 and GH-20104, instead declaring the old macros deprecated for 3.11 and remove them in 3.13

I personally agree with Irit, voting +1 for Option 3 since the old macros were soft-deprecated already by introducing new macros in 3.8, and more importantly made incompatible with pre-3.8 usage.

+1. The deprecation should follow PEP 387, which means emitting a warning. I think a compiler warning is appropriate; that could be done by having the macro call a deprecated function. Depending on how broken the macros are, a compiler warning could be backported to older versions.

See also: * https://bugs.python.org/issue44881 "Consider integration of PyObject_GC_UnTrack() with the trashcan C API". * https://bugs.python.org/issue44897 "Integrate trashcan mechanism into _Py_Dealloc" The issue44897 would make all these macros useless ;-) Victor

On 18 Aug 2021, at 11:47, Irit Katriel wrote:

It remains the decide how to backport the compiler warning to older versions, and to which versions.

3.8 is when the macros broke. Since they cause segfaults, I'd say that's a pretty good reason to backport the warnings. While 3.8 in security-only fix mode, I think there's no issue with showing the "pending" deprecation in the to-be-released 3.8.12. And if that, then 3.9.7, and 3.10.0rc2 as well. Of course, for the latter we need Pablo's confirmation. The additional argument for it is that we backported GH-27683 (using new macros in frameobject.c) all the way back to 3.8. So building Python itself shouldn't start raising the deprecation warnings itself. But if it does, we'll just fix that as well. - Ł

On Thu, Aug 19, 2021 at 11:28 AM Duncan Grisby

Thanks. Victor Stinner has now commented on the defect I raised to say that it is actually a bug in the traceback module, in that it should not be using Py_DECREF at all inside a signal handler. The fact that it leads to use of the old trashcan macros is not the cause of the crash that I saw.

I therefore don't think it's necessary for me to check with the updated frameobject.c. Do you agree? I can try to reproduce the situation, but I can't just quickly test it because the infrastructure where the crash was seen is now being used for other things.

Well, in order to verify that we fixed the problem you reported we need to be able to test the fix and see that the problem is gone. Right now we have two theories of what the cause could be, because there are two known bugs in that area. But you may have stumbled upon an unrelated third bug. The best scenario is that we close a bpo issue because we can reproduce the bug and see that it has been fixed. When that's not possible, there will come a point where we will close it and say "the code has changed since you observed the problem and we fixed things that could have caused it. Please open a new issue if you still see this bug".

On 26.04.21 23:55, Irit Katriel via Python-Dev wrote:

Re https://bugs.python.org/issue40608 https://bugs.python.org/issue40608.

I think it will be an act of kindness to deprecate Py_TRASHCAN_SAFE_BEGIN/END in 3.10 and tell people to use Py_TRASHCAN_BEGIN/END instead.

TL;DR: There was a change in 3.8 that introduced the latter while leaving the former for backwards compatibility, but also inadvertently breaking them. This is not an easy bug to deal with in the wild, we found it because we have a unit test in our codebase referencing https://bugs.python.org/issue16602 http://bugs.python.org/issue16602.  A deprecation note pointing to the new macros would have made it easier.

Is there any reason not to deprecate the old macros?

Just a note (I'm happy that this old code is still in use :) ), can't we think of replacing it somehow by functions in the case of the Limited API? The API is so often used that it would make sense to _always_ don't crash deeply nested structures. Or do you think it makes no sense at all? Then let's turn it into a no-op. But the current mixed situation is not really pleasant. -- Christian Tismer-Sperling :^) tismer@stackless.com Software Consulting : http://www.stackless.com/ Strandstraße 37 : https://github.com/PySide 24217 Schönberg : GPG key -> 0xFB7BEE0E phone +49 173 24 18 776 fax +49 (30) 700143-0023