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

[MAC] Fix LogicalLoraChannelHelper Object double destruction #180

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

non-det-alle
Copy link
Collaborator

Bug explanation: SetLogicalLoraChannelHelper() assigned a plain LogicalLoraChannelHelper Object (i.e., not a Ptr) to LorawanMac's member. Objects do not have a dedicated assignment operator implementation so their members are copied without modifications. Objects internally store a pointer to a buffer of aggregated objects. When the copy-assigned function parameter Object goes out of scope, its destructor is called, clearing the buffer of aggregated objects stored inside. The same buffer address was also stored in the LorawanMac's member by the copy assignment. At LorawanMac destruction time, this causes a second attempt at clearing the buffer, accessing already unallocated memory.

Note: The examples do not end up in a segmentation fault because currently ref counting pointers are not correctly released at the end of the simulation, effectively causing a memory leak. In the future we must implement DoDispose.

Note2: Found this while trying to implement MAC layer tests for coverage of #179.

Bug explanation: SetLogicalLoraChannelHelper() assigned a plain LogicalLoraChannelHelper Object (i.e., not a Ptr) to LorawanMac's member. Objects do not have a dedicated assignment operator implementation so their members are copied without modifications. Objects internally store a pointer to a buffer of aggregated objects. When the copied function parameter Object goes out of scope, its destructor is called, clearing the buffer of aggregated objects stored inside. The same buffer address was also stored in the LorawanMac's member by the copy assignment. At LorawanMac destruction time, this causes a second attempt at clearing the buffer, accessing already unallocated memory.

Note: The examples do not end up in a segmentation fault because currently ref counting pointers are not correctly released at the end of the simulation, effectively causing a memory leak. In the future we must implement DoDispose.
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 85.29412% with 5 lines in your changes missing coverage. Please review.

Project coverage is 77.66%. Comparing base (82ea58a) to head (34f226c).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
model/end-device-lorawan-mac.cc 61.53% 5 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #180   +/-   ##
========================================
  Coverage    77.66%   77.66%           
========================================
  Files           66       66           
  Lines         6831     6831           
========================================
  Hits          5305     5305           
  Misses        1526     1526           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@non-det-alle non-det-alle requested a review from pagmatt October 23, 2024 09:30
@non-det-alle non-det-alle removed the request for review from pagmatt November 5, 2024 12:51
@non-det-alle non-det-alle merged commit c1bd68b into develop Nov 5, 2024
20 checks passed
@non-det-alle non-det-alle deleted the double-destruct branch November 5, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant