Allow resource owners to access subsidiary portal memory during cleanup#46
Open
Allow resource owners to access subsidiary portal memory during cleanup#46
Conversation
During AbortTransaction/AbortSubTransaction we free portal child memory contexts, aka subsidiary contexts, ahead of freeing the portal memory itself, in order to not keep short-lived contexts like the executor context around longer than needed, e.g. in the case of holdable cursors. However, we currently free such memory when we first mark the portal as aborted, and before calling ResourceOwnerRelease. This presents a challenge for any resource owner that wants to access memory that was allocated inside the portal (e.g. with the executor context) and is registered to a resource owner that wants to use it during clean up. The current workaround is to allocate such memory in the TopMemoryContext, and use precise pfree(..) calls in both abort and success cases to avoid leaks. Instead, separate marking portals as aborted (and calling the cleanup hook) from releasing subsidiary memory contexts. Delay releasing the memory contexts until ResourceOwnerRelease has been called for all phases. For example, this allows allocating memory in the per-resource context and then accessing it when cleaning up a resource. Since abort handling is not supposed to allocate new memory (the pre-allocated TransactionAbortContext exists for that purpose), this should have limited performance impact in practice, and will allow both potential refactorings of current resource owners, and avoid complexity in future patches. Author: Lukas Fittl <lukas@fittl.com> Reviewed By: Discussion:
e4243ea to
dcc1df9
Compare
This removes the unnecessary use of TopMemoryContext for objects that need to be accessed during resource cleanup, instead allocating them in the current memory context. This is made possible thanks to the recent refactoring that delayed freeing portal subsidiary memory until after ResourceOwnerRelease has completed. The current memory context is assumed to be a child context of the currently active portal (if any), like the executor context, or another kind of local context that survives until after res owner cleanup. Author: Lukas Fittl <lukas@fittl.com> Reviewed By: Discussion:
dcc1df9 to
2959531
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TODO
Resources
395249e#diff-a6ae5f824a5e69f7be943cf2e596285247d645512c773dee0c3b68533d350676 (commit that added this early freeing of subsidiary portal memory)