Python 3.13b repeatedly setting superclass attribute in subclass leads to crashes #119462
Assignees
Labels
Comments
The-Compiler
added
the
type-crash
|
I did a quick check for recent changes to It would help a great deal if you could bisect the problem to a particular edit or construct a minimal reproducer that excludes third-party extensions. |
|
I got it down from 15 minutes to 7 minutes runtime, but at this point, even just removing a handful of test files from the run makes it not reproduce anymore, from what I've tried. I don't think a minimal example is realistic at this point given that I can only get it to trigger after running >3000 test cases, but I'll keep trying to reduce it a bit more, and then do a git bisect. It will probably take me few days, unless I can get things reliable enough for a |
|
Quick status update: After trying to reduce the test files being run one by one, I found a more minimal combination that still reliably triggers the bug: tox -e py313-pyqt66 -- \
tests/unit/browser/test_browsertab.py \
tests/unit/browser/test_caret.py \
tests/unit/browser/test_downloadview.py \
tests/unit/browser/test_history.py \
tests/unit/browser/test_hints.py \
tests/unit/browser/test_navigate.py \
tests/unit/config/test_configdata.py \
tests/unit/config/test_configtypes.py \
tests/unit/config/test_configinit.py \
tests/unit/config/test_configfiles.py \
tests/unit/mainwindow/test_messageview.py -v -sthat's still almost 2000 tests with runtime of 4 minutes though. I'll now proceed to bisecting CPython and hope that'll help find the culprit. |
|
Bisected to 992446d:
Not sure what to make of this. The change looks quite innocent, but I've double-checked it's indeed that commit that causes the memory corruption to happen. cc @markshannon |
|
I got it to fail under valgrind with Valgrind reports: I'll play around with |
|
Similar result with ASan: |
|
I've finally been able to reduce this a lot at least within the qutebrowser project - those two tests trigger the bug as soon as the second one runs for the 1010th time. import pytest
from qutebrowser.config import configdata, configtypes, configdata
from qutebrowser.utils import standarddir
def test_crash_1(qapp):
standarddir.init(None)
configdata.init()
configtypes.FontBase.set_defaults(None, '10pt')
@pytest.mark.parametrize("i", range(1010))
def test_crash_2(config_stub, i):
configtypes.Font().to_py("10pt default_family")I've not yet been able to reproduce this outside of pytest (or qutebrowser), as it still seems to be pretty sensitive about what's going on before the bug gets triggered (probably because gc related?). But at this point it looks like I should be able to cook up a minimal-ish example with a couple more hours of try and error. |
|
Aaaand I arrived at a minimal example: from PyQt6.QtCore import QSysInfo
def maybe_crash():
class StringHolder:
value = None
@classmethod
def set_value(cls):
# needs to be set here, setting from outside doesn't trigger the crash.
# anything that returns a QString from Qt/C++
cls.value = QSysInfo.productType()
class StringHolderSub(StringHolder):
# needs to be subclass, using StringHolder directly to access .value
# doesn't trigger the crash.
pass
for _ in range(1010): # triggers exactly after 1010 times.
StringHolder.set_value()
StringHolderSub.value
if __name__ == "__main__":
for _ in range(5):
# crash is not 100% reproducible with the minimal reproducer
maybe_crash()Crashes reliably when using a @rhettinger @markshannon Hope that works? It still requires PyQt6, I have not tested yet if the string can also be from another third-party library. A normal Python string won't trigger it. Sorry for the notification-heavy notes to myself here - since this was a longer process, I figured it'd be better to have my notes here than just for myself. |
|
|
Is the string you are creating escaping to other code before it is fully initialized? Can you link to the source of |
|
I ran a |
|
I can reproduce the bug in a reliable way without PyQt with a debug build of Python: def maybe_crash():
class StringHolder:
value = None
@classmethod
def set_value(cls):
cls.value = b'abc'.decode()
class StringHolderSub(StringHolder):
pass
for _ in range(1010):
StringHolder.set_value()
StringHolderSub.value
if __name__ == "__main__":
for _ in range(5):
maybe_crash()Example of output: |
vstinner
changed the title
|
If I revert this change on the main branch, I can no longer reproduce the bug: diff --git a/Include/cpython/object.h b/Include/cpython/object.h
index 0ab94e5e2a..0bfc20ac9c 100644
--- a/Include/cpython/object.h
+++ b/Include/cpython/object.h
@@ -229,7 +229,6 @@ struct _typeobject {
/* bitset of which type-watchers care about this type */
unsigned char tp_watched;
- uint16_t tp_versions_used;
};
/* This struct is used by the specializer
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 958f42430c..333ddb811c 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -1214,8 +1214,6 @@ _PyType_GetVersionForCurrentState(PyTypeObject *tp)
-#define MAX_VERSIONS_PER_CLASS 1000
-
static int
assign_version_tag(PyInterpreterState *interp, PyTypeObject *type)
{
@@ -1232,10 +1230,6 @@ assign_version_tag(PyInterpreterState *interp, PyTypeObject *type)
if (!_PyType_HasFeature(type, Py_TPFLAGS_READY)) {
return 0;
}
- if (type->tp_versions_used >= MAX_VERSIONS_PER_CLASS) {
- return 0;
- }
- type->tp_versions_used++;
if (type->tp_flags & Py_TPFLAGS_IMMUTABLETYPE) {
/* static types */
if (NEXT_GLOBAL_VERSION_TAG > _Py_MAX_GLOBAL_TYPE_VERSION_TAG) { |
|
Thanks @vstinner, that's really helpful. |
markshannon
changed the title
markshannon
changed the title
markshannon
added a commit
to faster-cpython/cpython
that referenced
this issue
Jun 19, 2024
markshannon
added a commit
that referenced
this issue
Jun 19, 2024
* Remove uses of Py_TPFLAGS_VALID_VERSION_TAG
markshannon
added a commit
to faster-cpython/cpython
that referenced
this issue
Jun 19, 2024
* Remove uses of Py_TPFLAGS_VALID_VERSION_TAG
|
Would it make sense to revert the change for now (in 3.13 and main branches), and consider a more long term approach in Python 3.14? |
markshannon
added a commit
that referenced
this issue
Jun 20, 2024
|
@The-Compiler can you confirm that this is fixed for you on both the main and 3.13 branches? |
It's fixed now (pending confirmation) |
|
I confirm that I can no longer reproduce the #119462 (comment) crash on the 3.13 development branch. I close the issue. |
github-project-automation
bot
moved this from Todo
to Done
in Release and Deferred blockers 🚫
|
The |
mrahtz
pushed a commit
to mrahtz/cpython
that referenced
this issue
Jun 30, 2024
* Remove uses of Py_TPFLAGS_VALID_VERSION_TAG
noahbkim
pushed a commit
to hudson-trading/cpython
that referenced
this issue
Jul 11, 2024
* Remove uses of Py_TPFLAGS_VALID_VERSION_TAG
estyxx
pushed a commit
to estyxx/cpython
that referenced
this issue
Jul 17, 2024
* Remove uses of Py_TPFLAGS_VALID_VERSION_TAG
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Crash report
What happened?
I'm trying to run the qutebrowser testsuite with Python 3.13, and am running into an issue where a test reproducibly fails (usually by crashing the interpreter), but only when I run the entire testsuite (not when run in isolation, or even just the tests in the same subfolder).
Given those circumstances, it seems tricky to get to a minimal example. I thought I'd open this issue in the hope of distilling things down further, and arriving at such an example. In the meantime, the best reproduction steps I can come up with are:
A few tests will fail with a
--with-pydebugbuild due to timeouts, those can be ignored. After a while (~13 minutes with--with-pydebugunder gdb on my system), one of the tests intests/unit/mainwindow/test_messageview.pywill fail, usually due to a failing assertion becausePyUnicode_KINDdid return an invalid value.Failing Python code:
stacktrace:
Sometimes I've also seen a
MemoryErroron the line callingstr.replace, or an ominous:with the following Python stack (note that jinja is involved, which might or might not be a trigger?):
qtbot = <pytestqt.qtbot.QtBot object at 0x7552ae9d6200>, view = <qutebrowser.mainwindow.messageview.MessageView object at 0x7552acd7fb10> config_stub = <qutebrowser.config.config.Config object at 0x7552acbe6170> def test_changing_timer_with_messages_shown(qtbot, view, config_stub): """When we change messages.timeout, the timer should be restarted.""" config_stub.val.messages.timeout = 900000 # 15s > view.show_message(message.MessageInfo(usertypes.MessageLevel.info, 'test')) tests/unit/mainwindow/test_messageview.py:172: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ qutebrowser/mainwindow/messageview.py:148: in show_message widget = Message.from_info(info) qutebrowser/mainwindow/messageview.py:71: in from_info return cls( qutebrowser/mainwindow/messageview.py:62: in __init__ stylesheet.set_register(self, qss, update=False) qutebrowser/config/stylesheet.py:32: in set_register observer.register() qutebrowser/config/stylesheet.py:97: in register qss = self._get_stylesheet() qutebrowser/config/stylesheet.py:86: in _get_stylesheet return _render_stylesheet(self._stylesheet) qutebrowser/config/stylesheet.py:41: in _render_stylesheet return template.render(conf=config.val) .tox/py313-pyqt66/lib/python3.13/site-packages/jinja2/environment.py:1304: in render self.environment.handle_exception() .tox/py313-pyqt66/lib/python3.13/site-packages/jinja2/environment.py:939: in handle_exception raise rewrite_traceback_stack(source=source) <template>:7: in top-level template code ??? qutebrowser/utils/jinja.py:117: in getattr return getattr(obj, attribute) qutebrowser/config/config.py:633: in __getattr__ return self._config.get(name) qutebrowser/config/config.py:385: in get return opt.typ.to_py(obj) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <qutebrowser.config.configtypes.Font completions=None none_ok=False>, value = 'default_size default_family' def to_py(self, value: _StrUnset) -> _StrUnsetNone: self._basic_py_validation(value, str) if isinstance(value, usertypes.Unset): return value elif not value: return None if not self.font_regex.fullmatch(value): # pragma: no cover # This should never happen, as the regex always matches everything # as family. raise configexc.ValidationError(value, "must be a valid font") if (value.endswith(' default_family') and self.default_family is not None): > value = value.replace('default_family', self.default_family) E TypeError: replace() argument 2 must be str, not + qutebrowser/config/configtypes.py:1244: TypeErrorwhich leads me to the conclusion that there must be some sort of memory corruption going on there.
On one run, I've also see a GC-related crash, which I'm not sure is related:
Python stack:
C stack:
I'm lost here on how to best debug this further. My best bet would be to try and at least get an example that I can run more quickly, and then try and bisect CPython in order to find the offending change. If there are any other guesses or approaches to debug what could be going on here, I'd be happy to dig in further.
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Output from running 'python -VV' on the command line:
Python 3.13.0b1 (main, May 23 2024, 09:21:12) [GCC 13.2.1 20240417]
Linked PRs
The text was updated successfully, but these errors were encountered: