gh-118331: Handle errors in _PyObject_SetManagedDict - #118334
Conversation
Sorry, something went wrong.
When detaching a dict, the copy_values call may fail due to out-of-memory errors. This can be triggered by test_no_memory in test_repl.
|
Running this on this branch: make clean
git clean -fdx .
./configure --with-pydebug && make -j10
./python.exe Lib/test/test_repl.pyI still get the error: .....F.
======================================================================
FAIL: test_no_memory (__main__.TestInteractiveInterpreter.test_no_memory)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/private/tmp/cpython/Lib/test/test_repl.py", line 86, in test_no_memory
self.assertIn(p.returncode, (1, 120))
~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: -6 not found in (1, 120)
----------------------------------------------------------------------
Ran 7 tests in 0.331s
FAILED (failures=1) |
Sorry, something went wrong.
|
Hmmm... maybe there are multiple unhandled exceptions? I haven't yet been able to reproduce additional failures with this PR. |
Sorry, something went wrong.
|
My experience is the same as @colesbury:
I tested on both fedora and MacOS. |
Sorry, something went wrong.
|
It's really strange, I can repro on main with two different Macs: my work one (which is at work until Monday) and this one. I can also repro with this branch and #118336. |
Sorry, something went wrong.
|
gh-118331 fixes it for me, and also with that merged into this. Thanks! |
Sorry, something went wrong.
DinoV
left a comment
There was a problem hiding this comment.
LGTM!
Sorry, something went wrong.
|
|
||
| err = _PyDict_DetachFromObject(dict, obj); | ||
| if (err == 0) { | ||
| FT_ATOMIC_STORE_PTR(_PyObject_ManagedDictPointer(obj)->dict, |
There was a problem hiding this comment.
If we're no longer assigning this before calling _PyDict_DetachFromObject we can bring back some of the assertions in _PyDict_DetachFromObject, specifically:
assert(_PyObject_ManagedDictPointer(obj)->dict == mp);
assert(_PyObject_InlineValuesConsistencyCheck(obj));
I think assigning after should be fine after all, because the object and the dict are locked, so we'd only see reads which are proceeding against the values which are being copied. Once the values are invalidated we'd go to the dict which is being detached, and then we'd finally start seeing the new dict.
Sorry, something went wrong.
When detaching a dict, the copy_values call may fail due to out-of-memory errors. This can be triggered by test_no_memory in test_repl.