classification
Title: PY3.8 GC segfault (Py_TRASHCAN_SAFE_BEGIN/END are not backwards compatible)
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: iritkatriel, jdemeyer, lukasz.langa, pablogsal, pitrou
Priority: normal Keywords: patch

Created on 2020-05-12 18:07 by iritkatriel, last changed 2021-08-27 12:16 by iritkatriel. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 20103 closed iritkatriel, 2020-05-15 08:26
PR 20104 closed iritkatriel, 2020-05-15 10:09
Messages (10)
msg368740 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2020-05-12 18:07
While migrating our codebase from python 3.7 to python 3.8, one of our unit tests segfaulted and I’ve narrowed it down to the change of typeobject.c in:

  commit 351c67416ba4451eb3928fa0b2e933c2f25df1a3
  Author: Jeroen Demeyer <J.Demeyer@UGent.be>
  Date:   Fri May 10 19:21:11 2019 +0200
  bpo-35983: skip trashcan for subclasses (GH-11841)   

It seems that Py_TRASHCAN_SAFE_BEGIN/END are not backwards compatible (as claimed in the comment above their definition in object.h).

This patch has a unit test that currently fails, along with the change in typeobject.c that restores the code that broke it: 

https://github.com/iritkatriel/cpython/commit/d962dd7f800fdaaeacd5748c6e3d38bf1f5053c1

I believe that this change needs to be pushed until the Py_TRASHCAN_SAFE_BEGIN/END are removed and all users are forced to migrate to the new Py_TRASHCAN_BEGIN/END.
msg368742 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2020-05-12 18:54
I need to check, but I think this is a duplicate of bpo-35983, which still has PR 12607 open.
msg377414 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2020-09-23 18:23
Have you had a chance to look into this?

It took some debugging to get to the bottom of this problem in our system (nothing in the "what's new in python 3.8" doc gave any hint). 

The solution I went for was to replace the old macros with the new ones. If it's not easy or desirable to fix the old macros, should they not be removed?
msg377427 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-09-23 21:41
Hi Irit,

Could you kindly check if PR 12607 solves this issue in any way?
msg377431 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2020-09-23 22:46
I've confirmed that my test still segfaults on master, but if I take the Py_TRASHCAN_SAFE_BEGIN(op) from PR 12607 then it doesn't segfault.
msg377438 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-09-24 07:12
I think that we should try to land PR 12607 first.
msg391870 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-04-25 22:32
There has been no progress on PR12607. I think we should at least deprecate Py_TRASHCAN_SAFE_BEGIN/END in 3.10 and tell people to use Py_TRASHCAN_BEGIN/END instead.
msg399683 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-08-16 21:26
Agreed with Irit. GH-12607 is conflicting with `main` and in an unclear review state. Irit's got both GH-20104 which fixes the segfault and GH-27693 which deprecates the old macros.

It's sad we missed the train for either GH-20104 or the original GH-12607 to get merged to 3.10 but it's a C API change so there's no discussion.

There's no hurry then in deciding for 3.11 if we leave the old macros as is and just deprecate them OR we both fix them using GH-20104 and deprecate them anyway. What are your thoughts, Pablo?
msg399685 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-08-16 21:32
Re GH-20104 - What I did there was to simply revert the change that git bisect showed caused my segfault.

It's been a year since then, and I don't understand this stuff well enough to say that reverting this brings us to a good state, with respect to everything else that has changed. So let's be careful about that.
msg400420 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-08-27 12:16
Superseeded by deprecation under Issue44874.
History
Date User Action Args
2021-08-27 12:16:36iritkatrielsetstatus: open -> closed
resolution: wont fix
messages: + msg400420

stage: patch review -> resolved
2021-08-16 21:32:51iritkatrielsetmessages: + msg399685
2021-08-16 21:26:46lukasz.langasetnosy: + lukasz.langa
messages: + msg399683
2021-04-26 21:16:18iritkatrielsetversions: + Python 3.8
2021-04-25 22:32:40iritkatrielsetmessages: + msg391870
2020-09-24 07:12:11pablogsalsetmessages: + msg377438
2020-09-23 22:46:28iritkatrielsetmessages: + msg377431
versions: + Python 3.9, Python 3.10, - Python 3.8
2020-09-23 21:41:04pablogsalsetmessages: + msg377427
2020-09-23 18:23:50iritkatrielsetmessages: + msg377414
2020-05-15 10:09:58iritkatrielsetpull_requests: + pull_request19412
2020-05-15 08:26:14iritkatrielsetkeywords: + patch
stage: patch review
pull_requests: + pull_request19411
2020-05-12 18:54:17jdemeyersetmessages: + msg368742
2020-05-12 18:07:29iritkatrielcreate