bpo-31626: Fix _PyObject_DebugReallocApi()#4310
Conversation
Sorry, something went wrong.
|
As part of efforts to make Python memory allocators "correct", I propose to "fix" _PyObject_DebugReallocApi(). For the rationale, see: |
Sorry, something went wrong.
Sorry, something went wrong.
|
@skrah: In the PR #3844, you wrote "Why the fatal error? If a shrinking realloc() fails, the old memory is still valid (just too large)." @serhiy-storchaka wrote a smart and correct implementation in PR #4210 for the master branch: save erased bytes, to be able to restore them on failure. But this implementation is complex. Our Python 3.6 release manager, @ned-deily, is against backporting this "correct" fix to Python 3.6: "My reaction is that the risk due to complexity of the changes outweigh the benefits so I would agree with @serhiy-storchaka that we should not backport this." In debug mode, Python 2 always erased bytes before calling realloc(). If realloc() fails on shrinking a memory block... the original memory block is left modfied (with erased bytes). My PR makes the code failing with a fatal error in this case which should never happen. So I propose to be optimistic and use the Python 2 code in Python 3.6, but fix the code to trigger a fatal error if realloc() fails whereas it should never fail :-) |
Sorry, something went wrong.
|
Once this PR will be merged, I will prepare a PR for Python 3.6. |
Sorry, something went wrong.
|
I'm not sure that it is worth to make the code crashing in maintained release. If the shrinking realloc() fails in real world, there is a chance that the third party code will just fail with MemoryError. Or will never read erased memory. With this change it will crash immediately. This doesn't look an enhancement or bug fix. |
Sorry, something went wrong.
_PyObject_DebugReallocApi() now calls Py_FatalError() if realloc() fails to shrink a memory block. Call Py_FatalError() because _PyObject_DebugReallocApi() erased freed bytes *before* realloc(), expecting that realloc() *cannot* fail to shrink a memory block.
|
@serhiy-storchaka: "If the shrinking realloc() fails in real world, there is a chance that the third party code will just fail with MemoryError. Or will never read erased memory." If the application uses the memory block after the realloc failure, the application can crash because of a bug in Python memory allocation. @serhiy-storchaka: "This doesn't look an enhancement or bug fix." realloc() must leave the memory block unchanged on failure. It's not the cause currently, so it's a bug and my change is a bug fix. |
Sorry, something went wrong.
|
I will merge this PR this week if nobody complains against it. |
Sorry, something went wrong.
Reviewers
5 participants
_PyObject_DebugReallocApi() now calls Py_FatalError() if realloc()
fails to shrink a memory block.
Call Py_FatalError() because _PyObject_DebugReallocApi() erased freed
bytes before realloc(), expecting that realloc() cannot fail to
shrink a memory block.
https://bugs.python.org/issue31626