GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-118362: Fix thread safety around lookups from the type cache in the face of concurrent mutators by DinoV · Pull Request #118454 · python/cpython Conversation
|
This is part of the reason why I assert: In type_setattro. Type objects will always have a dictptr, so we'll either go to
|
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Fix some formatting Add news blurb Use PyDictObject * more Rename _PyDict_GetItemRef_LockHeld to _PyDict_GetItemRef_Unicode_LockHeld Reduce iterations in test Expose _PyObject_SetAttributeErrorContext for attaching context
|
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| extern PyTypeObject* _PyType_CalculateMetaclass(PyTypeObject *, PyObject *); | ||
| extern PyObject* _PyType_GetDocFromInternalDoc(const char *, const char *); | ||
| extern PyObject* _PyType_GetTextSignatureFromInternalDoc(const char *, const char *, int); | ||
| extern int _PyObject_SetAttributeErrorContext(PyObject* v, PyObject* name); |
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.
| extern int _PyObject_SetAttributeErrorContext(PyObject* v, PyObject* name); | |
| extern int _PyObject_SetAttributeErrorContext(PyObject *v, PyObject *name); |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
-
👍 1 reaction
| PyObject *dict = lookup_tp_dict(_PyType_CAST(base)); | ||
| assert(dict && PyDict_Check(dict)); | ||
| res = _PyDict_GetItem_KnownHash(dict, name, hash); | ||
| if (_PyDict_GetItemRef_KnownHash((PyDictObject *)dict, name, hash, &res) < 0) { |
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.
I think the PyErr_Occurred() check below can be removed now.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
-
👍 1 reaction
colesbury
left a comment
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.
LGTM with a few minor issues:
- We need to handle the
PyDict_New()returningNULLcase intype_setattro - Minor formatting in
_PyObject_SetAttributeErrorContext PyErr_Occurred()case is no longer need infind_name_in_mro
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
|
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| class A: | ||
| attr = 1 | ||
|
|
||
| @unittest.skipIf(is_wasi, "WASI has no threads.") |
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.
Oh, probably better to use @threading_helper.requires_working_threading()
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
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.
iOS and Android do both have working threading, but they don't support subprocesses. In theory multiprocessing.dummy.Pool should work on these platforms, but in practice it doesn't because it imports too much of the main multiprocessing module.
The simplest solution is probably to use concurrent.futures.ThreadPoolExecutor instead.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
-
👍 2 reactions
… in the face of concurrent mutators (python#118454) Add _PyType_LookupRef and use incref before setting attribute on type Makes setting an attribute on a class and signaling type modified atomic Avoid adding re-entrancy exposing the type cache in an inconsistent state by decrefing after type is updated
Python 3.14 introduced a new assertion that prevents us from using PyObject_GenericSetAttr directly in our meta type. To work around this, we manipulate the type dict directly. This workaround is a simplified variant of Cython's workaround from cython/cython#6325. The relevant Python change is in python/cpython#118454
Python 3.14 introduced a new assertion that prevents us from using PyObject_GenericSetAttr directly in our meta type. To work around this, we manipulate the type dict directly. This workaround is a simplified variant of Cython's workaround from cython/cython#6325. The relevant Python change is in python/cpython#118454
Python 3.14 introduced a new assertion that prevents us from using PyObject_GenericSetAttr directly in our meta type. To work around this, we manipulate the type dict directly. This workaround is a simplified variant of Cython's workaround from cython/cython#6325. The relevant Python change is in python/cpython#118454
Python 3.14 introduced a new assertion that prevents us from using PyObject_GenericSetAttr directly in our meta type. To work around this, we manipulate the type dict directly. This workaround is a simplified variant of Cython's workaround from cython/cython#6325. The relevant Python change is in python/cpython#118454
Python 3.14 introduced a new assertion that prevents us from using PyObject_GenericSetAttr directly in our meta type. To work around this, we manipulate the type dict directly. This workaround is a simplified variant of Cython's workaround from cython/cython#6325. The relevant Python change is in python/cpython#118454
Python 3.14 introduced a new assertion that prevents us from using PyObject_GenericSetAttr directly in our meta type. To work around this, we manipulate the type dict directly. This workaround is a simplified variant of Cython's workaround from cython/cython#6325. The relevant Python change is in python/cpython#118454
Python 3.14 introduced a new assertion that prevents us from using PyObject_GenericSetAttr directly in our meta type. To work around this, we manipulate the type dict directly. This workaround is a simplified variant of Cython's workaround from cython/cython#6325. The relevant Python change is in python/cpython#118454
* Initial 3.14 commit (cherry picked from commit caac33d) * Apply alignment fix (cherry picked from commit e10d333) * Disable problematic GC tests (cherry picked from commit e976558) * Set ht_token to NULL in Python 3.14 (cherry picked from commit 65af098) * Workaround for blocked PyObject_GenericSetAttr in metatypes Python 3.14 introduced a new assertion that prevents us from using PyObject_GenericSetAttr directly in our meta type. To work around this, we manipulate the type dict directly. This workaround is a simplified variant of Cython's workaround from cython/cython#6325. The relevant Python change is in python/cpython#118454 (cherry picked from commit 08550d0) * Use PyThreadState_GetUnchecked on Python 3.13 (cherry picked from commit f3face0) * Remove deprecated function call (cherry picked from commit 8dfe408) * Assign True instead of None to __clear_reentry_guard__ Not at all sure why this helps, but when assigning `None` instead, the object is gone at the time of garbage collection. (cherry picked from commit 8e0333d) * Move tp_clear workaround to .NET In Python 3.14, the objects __dict__ seems to already be half deconstructed, leading to crashes during garbage collection. Since gc in Python is single-threaded (I think :)), it should be fine to have a single static for this. If that is not true, we can always use a thread-local instead. (cherry picked from commit 908e13b) * Use non-BOM encodings (pythonnet#2370) * Use non-BOM encodings The documentation of the used `PyUnicode_DecodeUTF16` states that not passing `*byteorder` or passing a 0 results in the first two bytes, if they are the BOM (U+FEFF, zero-width no-break space), to be interpreted and skipped, which is incorrect when we convert a known "non BOM" string, which all strings from C# are. (cherry picked from commit 195cde6) * Preserve SyntaxError source line in message on Python 3.12+ Python 3.12 eagerly normalizes the error indicator, so PyErr_Fetch now hands us the SyntaxError instance (whose str() omits the offending source line) instead of the raw args tuple (whose str() included it). Callers that surface PythonException.Message for compile diagnostics therefore lost the offending source text on 3.12+. GetMessage now re-appends the SyntaxError 'text' attribute when present. This is a no-op on <=3.11 (there the fetched value is a tuple without the SyntaxError attributes) and only affects SyntaxError messages. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Make embed tests compatible with Python 3.12+ behavior changes Three CPython behavior changes surfaced as failures/crashes once the overload-resolution crash was fixed, all on 3.12+: - ClassManagerTests.BindsCorrectOverloadForClassName crashed the host with "Python memory allocator called without holding the GIL". TestClass2's Get(PyObject o) re-enters Python via ToPython() while MethodBinder has released the GIL (allow_threads) around the managed call. A managed callback that re-enters Python must re-acquire the GIL; tolerated on <=3.11, fatal on 3.12+. Wrap the body in using (Py.GIL()). - TestGetsPythonCodeInfoInStackTrace[ForNestedInterop]: 3.12+ adds caret indicator lines (e.g. "~~~^^^") under source lines in tracebacks, shifting the positional assertions. Drop caret-only lines before asserting (no-op on <=3.11). - Codecs.ExceptionDecodedNoInstance: 3.12 eagerly normalizes exceptions, so the error indicator always carries an instance ("value"); the instanceless scenario this decoder targets can no longer be produced. Guard the test to <3.12. Verified: full embed suite green on 3.11 (910/910) with these changes; the three previously-failing tests pass on 3.14. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Add Python 3.12 / 3.13 ABI offsets and CI jobs The fork resolves PyTypeObject field offsets from the hardcoded TypeOffset{major}{minor} tables (it does not run geninterop at build), so a missing table makes ABI.Initialize throw "Python ABI v... is not supported" and every test on that version fails at PythonEngine init. Only 3.6-3.11 and 3.14 tables were present. Vendor the 3.12 and 3.13 tables from pythonnet/pythonnet upstream (byte-identical to upstream master; same source as the already-present TypeOffset314) and add 3.12 + 3.13 to the CI matrix. Local verification (uv standalone CPython 3.13, this branch's fixes): embed suite ABI-initializes correctly and runs 847 passed / 0 failed (parity with 3.14). 3.12 table is vendored from the same authoritative source; CI exercises it. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Support Python 3.13/3.14 re-init in tests; drop obsolete TestDomainReload The embed-test suite re-initializes the interpreter per fixture. On CPython 3.13/3.14 the suite aborted with "Failed to import encodings module" - not a filesystem problem (strace shows the file opens fine) but interpreter import state corrupted across re-initialization. Root cause isolated to a single test: TestPythonEngineProperties.SetPythonPath. It uses PythonEngine.PythonPath, which pins a fixed module search path via the deprecated Py_SetPath. CPython 3.13+ keeps that path config in _PyRuntime across Py_Finalize and offers no way to reset it back to auto-computation without the PyConfig API, so once this test runs every later re-initialization in the same process is forced onto the pinned path and eventually cannot bootstrap encodings. All other fixtures - including the normal Initialize/Shutdown cycles in pyinitialize and TestFinalizer - run fine in a single process. Run only SetPythonPath in its own test process so it cannot pollute the rest of the suite. Verified locally: full embed suite green on 3.11, 3.13 and 3.14 (main run 908 / SetPythonPath 1, 0 failures); no regression. Also delete TestDomainReload: AppDomain reload is not supported on modern .NET (single-domain), so those tests (MarshalByRefObject / AppDomain.CreateDomain) are obsolete. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --------- Co-authored-by: Benedikt Reinartz <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
4 participants
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.