You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The reason will be displayed to describe this comment to others. Learn more.
Thanks @garbear - would you mind looking at the issue I created and that others have added to which pre-dates this commit or do you need me to create a fresh issue here? I am happy to if needed.
The reason will be displayed to describe this comment to others. Learn more.
It looks like the segfault is a separate issue. This change fixes a hang (not a crash) that happens in Py_Finalize(). I don't see that function entered in the stack traces on the issue you linked.
Though it's possible that explicitly clearing the modules works around the Python bug somehow.
Looks like it's crashing when trying to garbage collect something when cleaning the module dict. So this commit doesn't fix the crash, it just makes it happen earlier in the code that I added, and shows exactly what the problem is.
According to ChatGPT:
The crash you're experiencing is likely due to the added PyDict_Clear(modules); line, which clears all the loaded Python modules' dictionaries, including potentially critical ones used by the interpreter itself. This could lead to undefined behavior, such as trying to untrack or finalize objects that have already been partially cleaned up.
Here's a refined approach to avoid the crash:
Ensure Proper Module Cleanup: Instead of clearing all the module dictionaries, carefully remove the references that might be causing the circular dependencies. You can iterate over the modules and selectively clear them.
Finalize Only When Safe: Ensure that no other Python-related operations are occurring while finalizing.
It recommends the following change:
if (Py_IsInitialized())
{
// Switch to the main interpreter thread before finalizingPyThreadState_Swap(PyInterpreterState_ThreadHead(PyInterpreterState_Main()));
// Clear modules safely
PyObject *modules = PyImport_GetModuleDict();
PyObject *key, *value;
Py_ssize_t pos = 0;
while (PyDict_Next(modules, &pos, &key, &value))
{
if (value != NULL && PyModule_Check(value))
{
PyObject *dict = PyModule_GetDict(value);
if (dict != NULL)
{
PyDict_Clear(dict);
}
}
}
Py_Finalize();
}
The added safety checks may fix the crash on Kodi's end. If not, it could be a problem with the interpreter, as I see lots of talk about backporting crash fixes.
The reason will be displayed to describe this comment to others. Learn more.
Can you test the change? The problem looks to be in the garbage collector, and while that change adds some safety it doesn't address any garbage-collector-specific problems. So I doubt the change will fix the problem, but it's worth a test. If things improve then I'll PR it.
The reason will be displayed to describe this comment to others. Learn more.
It looks like a Python issue in the garbage collector, possibly an issue that arises when we transitioned to multiple interpreters. The commit here doesn't expose the issue, it just makes it occur sooner, so I'm not sure what I can do. Have you tried backporting all the crash fixes I've seen go upstream?
The reason will be displayed to describe this comment to others. Learn more.
Hi guys
FYI, i patched python 3.12.3 with PR python/cpython#118618
and it seems to have fixed the issue for me
the segfault on exit was systematic before
tried garbear's patch : fail (could patch and build but crashed on exit)
tried python PR's patch on 3.12.4 : fail (could not patch)
tried python PR's patch on 3.12.3 : fail on ABI changes (could not patch)
removed changes concerning Doc/data/python3.12.abi from the patchfile and it applied successfully
rebuilt python with that patch & kodi (without garbear's patch) => i have not been able to produce the issue since
here's the patchfile for python 3.12.3 if anybody needs it gh-118618-use-pointer-for-interp-obmalloc-state.patch.gz
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@garbear - Unfortunately, this commit does not fix the issue at at least on Arch Linux (python 3.12.4-1).
Do you need me to open a fresh issue?
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the segfault caused by
PyDict_Clear()
? Go ahead and open an issue and I'll look into it.0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @garbear - would you mind looking at the issue I created and that others have added to which pre-dates this commit or do you need me to create a fresh issue here? I am happy to if needed.
https://gitlab.archlinux.org/archlinux/packaging/packages/python/-/issues/11
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the segfault is a separate issue. This change fixes a hang (not a crash) that happens in Py_Finalize(). I don't see that function entered in the stack traces on the issue you linked.
Though it's possible that explicitly clearing the modules works around the Python bug somehow.
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@garbear - all of the data including the stack traces from @jelly in that report is on kodi before this commit was incorporated.
@jelly - now that kodi-20-7 includes this commit and has hit the repos, would you mind testing on your system to confirm my findings and potentially providing another stack trace as you did in https://gitlab.archlinux.org/archlinux/packaging/packages/python/-/issues/11
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@garbear - is this helpful?
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I see:
Looks like it's crashing when trying to garbage collect something when cleaning the module dict. So this commit doesn't fix the crash, it just makes it happen earlier in the code that I added, and shows exactly what the problem is.
According to ChatGPT:
It recommends the following change:
The added safety checks may fix the crash on Kodi's end. If not, it could be a problem with the interpreter, as I see lots of talk about backporting crash fixes.
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Are you planning to commit the code you're proposing? I am happy to build/test.
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you test the change? The problem looks to be in the garbage collector, and while that change adds some safety it doesn't address any garbage-collector-specific problems. So I doubt the change will fix the problem, but it's worth a test. If things improve then I'll PR it.
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you build Kodi with this patch?
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still has the problem:
and
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a Python issue in the garbage collector, possibly an issue that arises when we transitioned to multiple interpreters. The commit here doesn't expose the issue, it just makes it occur sooner, so I'm not sure what I can do. Have you tried backporting all the crash fixes I've seen go upstream?
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, if you can point them out, I can try building python 3.12.4 + these fixes.
0f5db1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi guys
FYI, i patched python 3.12.3 with PR python/cpython#118618
and it seems to have fixed the issue for me
the segfault on exit was systematic before
running gentoo + python 3.12.3 (3.12.3-r1) + kodi 21.0 (21.0-r2)
tried garbear's patch : fail (could patch and build but crashed on exit)
tried python PR's patch on 3.12.4 : fail (could not patch)
tried python PR's patch on 3.12.3 : fail on ABI changes (could not patch)
removed changes concerning Doc/data/python3.12.abi from the patchfile and it applied successfully
rebuilt python with that patch & kodi (without garbear's patch) => i have not been able to produce the issue since
here's the patchfile for python 3.12.3 if anybody needs it
gh-118618-use-pointer-for-interp-obmalloc-state.patch.gz
EDIT: i reworked the patch file (it was a CRLF issue in the patch process) i can now apply the patch for the full PR!
here are both patch files i used
gh-118618-use-pointer-for-interp-obmalloc-state.patch.tar.gz