-
-
Notifications
You must be signed in to change notification settings - Fork 31.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-118331: Handle errors in _PyObject_SetManagedDict #118334
Conversation
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.py I 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) |
Hmmm... maybe there are multiple unhandled exceptions? I haven't yet been able to reproduce additional failures with this PR. |
My experience is the same as @colesbury:
I tested on both fedora and MacOS. |
It's really strange, I can repro on I can also repro with this branch and #118336. |
gh-118331 fixes it for me, and also with that merged into this. Thanks! |
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!
|
||
err = _PyDict_DetachFromObject(dict, obj); | ||
if (err == 0) { | ||
FT_ATOMIC_STORE_PTR(_PyObject_ManagedDictPointer(obj)->dict, |
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.
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.
When detaching a dict, the copy_values call may fail due to out-of-memory errors. This can be triggered by
test_no_memory
intest_repl
.