Managed dealloc: managed buffers#2114
Merged
andrei-marinica merged 23 commits intofeat/mdropfrom Mar 16, 2026
Merged
Conversation
|
Contract comparison - from be956b5 to 25fdcbe
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements managed deallocation for managed buffers, enabling ManagedBuffer to properly free its underlying VM-side storage when dropped. Key changes include making read_from_payload unsafe (due to double-drop risks), adding a Drop impl for ManagedVec that iterates and drops items when needed, and introducing a memory benchmarking tool.
Changes:
- Enabled
ManagedBuffer::dropto calldrop_managed_buffer, with corresponding safety changes (unsafe fn read_from_payload,requires_drop()trait method,ManagedVec::Dropimpl) - Converted
to_arg_buffer()usages tointo_arg_buffer()to avoid cloning, and usedManagedRef/ManagedRefMutto wrap borrowed handles without triggering deallocation - Added
managed-mem-benchtool and extensiveHandleMapunit tests
Reviewed changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| framework/base/src/types/managed/basic/managed_buffer.rs | Enable buffer deallocation in Drop, update clone |
| framework/base/src/types/managed/wrapped/managed_vec_item.rs | Make read_from_payload unsafe, add requires_drop() and temp_decode |
| framework/base/src/types/managed/wrapped/managed_vec.rs | Add Drop impl, fix forget_into_handle, update PartialEq and get_unsafe |
| framework/base/src/types/managed/wrapped/managed_vec_ref.rs | Soft-dealloc via save_to_payload in drop |
| framework/base/src/types/managed/wrapped/managed_vec_ref_mut.rs | Updated drop comment and structure |
| framework/base/src/types/managed/wrapped/managed_vec_iter_owned.rs | Add unsafe to read_from_payload calls |
| framework/base/src/types/managed/wrapped/encoded_managed_vec_item.rs | Unsafe call with TODO |
| framework/base/src/types/managed/multi_value/multi_value_encoded.rs | Add into_arg_buffer, update to_arg_buffer to clone |
| framework/base/src/api/managed_types/managed_type_api_impl.rs | Add requires_managed_type_drop default |
| framework/base/src/storage/storage_set.rs | Use ManagedRef for temp handles |
| framework/base/src/contract_base/wrappers/blockchain_wrapper.rs | Use ManagedRef for temp handles |
| framework/base/src/contract_base/wrappers/storage_raw_wrapper.rs | Use new_uninit for temp buffer |
| framework/derive/src/managed_vec_item_derive.rs | Make derived read_from_payload unsafe |
| chain/vm/src/host/context/managed_type_container/handle_map.rs | Add assert on remove, shrink heuristic, extensive tests |
| chain/vm/src/host/context/managed_type_container/tx_managed_buffer.rs | Commented-out debug print |
| framework/scenario/src/api/impl_vh/*.rs | Explicit dispatcher drop ordering, debug println |
| framework/scenario/tests/*.rs | Update tests for unsafe read_from_payload / temp_decode |
| tools/managed-mem-bench/ | New memory leak detection tool |
| contracts/feature-tests/composability/**/src/*.rs | to_arg_buffer() → into_arg_buffer() |
| Various token payment files | Make read_from_payload unsafe |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
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.
Pull request overview
This PR implements managed deallocation for managed buffers, enabling
ManagedBufferto properly free its underlying VM-side storage when dropped. Key changes include makingread_from_payloadunsafe (due to double-drop risks), adding aDropimpl forManagedVecthat iterates and drops items when needed, and introducing a memory benchmarking tool.Changes:
ManagedBuffer::dropto calldrop_managed_buffer, with corresponding safety changes (unsafe fn read_from_payload,requires_drop()trait method,ManagedVec::Dropimpl)to_arg_buffer()usages tointo_arg_buffer()to avoid cloning, and usedManagedRef/ManagedRefMutto wrap borrowed handles without triggering deallocationmanaged-mem-benchtool and extensiveHandleMapunit tests