Skip to content
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

Improve a potential out-of-memory edge case in CowData by propagating errors out of _copy_on_write. #100672

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ivorforce
Copy link
Contributor

@Ivorforce Ivorforce commented Dec 20, 2024

I am not making much progress on #100619, so I'm making another small PR about an inconsistency in the CowData class that I noticed while spending so much quality time with it.

It's an edge case where _copy_on_write fails to allocate a new buffer (out of memory).
Previously, the error was logged, the non-1 refcount was ignored, and the shared data may have been reallocated in later instructions, possibly invalidating it for other owners.
Instead, this PR changes behavior such that an out-of-memory error is recognized and propagated upwards, such that no shared data is invalidated.

This is unlikely to have an effect during most runs, but it may improve error behavior in very rare edge cases.

The return value of _copy_on_write was previously ignored in all other spots, and still is with the new code. In those cases, it is likely that shared data is overwritten as a result (though not completely invalidated as in the resize case).

@Ivorforce Ivorforce requested a review from a team as a code owner December 20, 2024 22:11
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.

2 participants