Skip to content

[WIP] bpo-31626: Rewrite _PyMem_DebugRawRealloc() #3898

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

Closed
wants to merge 1 commit into from
Closed

[WIP] bpo-31626: Rewrite _PyMem_DebugRawRealloc() #3898

wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Oct 5, 2017

Previously, _PyMem_DebugRawRealloc() expected that the old memory
block remains readable and unchanged after realloc(). This assumption
is wrong on OpenBSD.

Rewrite _PyMem_DebugRawRealloc() as malloc()+free() rather than
realloc(). Debug hooks are reused. The old memory block remains valid
after the new memory block was allocated, so bytes can be safely
copied in a portable way.

The drawback is that _PyMem_DebugRawRealloc() now allocates a new
memory block while the old memory block remains allocated, until the
old memory block is freed, so the peak memory usage can be the double
in the worst case.

The second drawback is that the system realloc() is no more used in
debug mode.

https://bugs.python.org/issue31626

Previously, _PyMem_DebugRawRealloc() expected that the old memory
block remains readable and unchanged after realloc(). This assumption
is wrong on OpenBSD.

Rewrite _PyMem_DebugRawRealloc() as malloc()+free() rather than
realloc(). Debug hooks are reused. The old memory block remains valid
after the new memory block was allocated, so bytes can be safely
copied in a portable way.

The drawback is that _PyMem_DebugRawRealloc() now allocates a new
memory block while the old memory block remains allocated, until the
old memory block is freed, so the peak memory usage can be the double
in the worst case.

The second drawback is that the system realloc() is no more used in
debug mode.
@vstinner
Copy link
Member Author

vstinner commented Oct 5, 2017

I tagged the PR as WIP because I dislike the second drawback of my change: "the system realloc() is no more used in debug mode".

A bug in system realloc() would be missed completely in debug mode with this change.

@serhiy-storchaka
Copy link
Member

I'm opposed to this change. The purpose of the debug allocator is to help with finding bugs in user code. But if realloc() always returns a new reference, this can hide bugs. Some bugs now could be reproduced only with non-debug allocator.

@vstinner
Copy link
Member Author

vstinner commented Nov 2, 2017

PR #3844 was merged, I abandon my change. @serhiy-storchaka also wrote a lighter version of my PR without allocating memory on the heap: allocate memory on the stack instead, PR #4210.

@vstinner vstinner closed this Nov 2, 2017
@vstinner vstinner deleted the realloc_debug branch November 2, 2017 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants