mw/com: Use pointer instead of copy for EventDataControlComposite#454
Draft
ahmed0mousa wants to merge 4 commits into
Draft
mw/com: Use pointer instead of copy for EventDataControlComposite#454ahmed0mousa wants to merge 4 commits into
ahmed0mousa wants to merge 4 commits into
Conversation
ahmed0mousa
force-pushed
the
ahmo_fix_samle_allocatee_ptr
branch
3 times, most recently
from
a818e1c to
35d61d0
Compare
ahmed0mousa
changed the title
ahmed0mousa
changed the title
ahmed0mousa
force-pushed
the
ahmo_fix_samle_allocatee_ptr
branch
4 times, most recently
from
7e056e5 to
4bae17f
Compare
SampleAllocateeTracker counts outstanding SampleAllocateePtr instances and terminates if any outlive the SkeletonEvent, symmetric with SampleReferenceTracker for subscribers. SampleAllocateeGuard is a move-only RAII type backed by utils::ScopeExit that decrements the tracker on destruction. Issue: SWP-62366
…in lola SampleAllocateePtr SampleAllocateePtr previously copied the EventDataControlComposite into an std::optional on every allocation. Replace the field with a non-owning raw pointer, the composite is owned by SkeletonEvent and guaranteed to outlive all SampleAllocateePtr instances via SampleAllocateeTracker. Delete copy/move on EventDataControlComposite with static_asserts to prevent use-after-free. Issue: SWP-62366
…SampleAllocateePtr lifetime Add SampleAllocateeTracker to SkeletonEventBase and pass a SampleAllocateeGuard into every Allocate() and Send() call down through the binding interfaces and implementations. The guard is stored inside the resulting SampleAllocateePtr so the tracker count stays elevated for as long as any SampleAllocateePtr is alive, aborting the application if the SkeletonEvent is destroyed first. Issue: SWP-62366
The C++ LolaSampleAllocateePtrBinding changed event_data_control from a CxxOptional<EventDataControlComposite> to a raw pointer. Update the Rust struct accordingly and replace the UniquePtr union variant with a MockBinding variant to match the mock_binding::SampleAllocateePtr memory layout. Issue: SWP-62366
ahmed0mousa
force-pushed
the
ahmo_fix_samle_allocatee_ptr
branch
from
4bae17f to
ab9677e
Compare
bemerybmw
requested changes
Jun 10, 2026
| /// any SampleAllocateePtr instances created from it. The SampleAllocateeTracker ensures this by terminating | ||
| /// if SampleAllocateePtr instances outlive the SkeletonEvent. Can only be a nullptr if SampleAllocateePtr is | ||
| /// default constructed. | ||
| EventDataControlComposite<>* event_data_control_ptr_; |
Contributor
There was a problem hiding this comment.
should have static asserts here that EventDataControlComposite is not movable / copyable.
| /// \details In production code, guards must be obtained via SampleAllocateeTracker::Allocate() so that | ||
| /// every allocation is tracked. Direct default construction produces a no-op guard and is therefore | ||
| /// only intended for use in tests, where a real SampleAllocateeTracker is not available. | ||
| SampleAllocateeGuard() = default; |
Contributor
There was a problem hiding this comment.
This class should use "score/scope_exit/scope_exit.h" for handling deallocation on destruction so that we're not reimplementing the behaviour here. You can also then remove the move tests if you add a static assert that ScopeExit is used. See "score/mw/com/impl/bindings/lola/transaction_log_registration_guard.h" and the test in this pr as an example: #268
| "//score/mw/com:__subpackages__", | ||
| ], | ||
| deps = [":common_rs"], | ||
| deps = [ |
Contributor
There was a problem hiding this comment.
Please ask @bharatGoswami8 or @darkwisebear to double check the rust changes
| /// In production, guards must be obtained from SampleAllocateeTracker::Allocate(); use MakeSampleAllocateePtr | ||
| /// with a guard instead. | ||
| template <typename T> | ||
| auto MakeSampleAllocateePtr(T ptr) noexcept -> SampleAllocateePtr<typename T::element_type> |
Contributor
There was a problem hiding this comment.
Can this now be renamed to MakeTestSampleAllocateePtr?
| /// \details This is a non-owning pointer. The SkeletonEvent owns the EventDataControlComposite and must outlive | ||
| /// any SampleAllocateePtr instances created from it. The SampleAllocateeTracker ensures this by terminating | ||
| /// if SampleAllocateePtr instances outlive the SkeletonEvent. Can only be a nullptr if SampleAllocateePtr is | ||
| /// default constructed. |
Contributor
There was a problem hiding this comment.
this comment is different to the latest state of https://cc-github.bmwgroup.net/swh/safe-posix-platform/pull/17115/. Can you please make sure that you ported the latest version?
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
2 participants
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.
Changed SampleAllocateePtr to hold pointer to SkeletonEvent's EventDataControlComposite
Added SampleAllocateeTracker/Guard to track outstanding allocations
SkeletonEvent terminates if destroyed while SampleAllocateePtr instances exist
Update unit tests
Fixes state synchronization issues and prevents dangling pointer access.
Issue: SWP-62366