From c1bd68b7de26b3421500bd6f299c65056c0b05c6 Mon Sep 17 00:00:00 2001 From: Alessandro Aimi Date: Tue, 5 Nov 2024 13:52:41 +0100 Subject: [PATCH] [MAC] Fix LogicalLoraChannelHelper Object double destruction (#180) 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. --- helper/lorawan-mac-helper.cc | 30 ++++++++++++------------- model/class-a-end-device-lorawan-mac.cc | 2 +- model/end-device-lorawan-mac.cc | 26 ++++++++++----------- model/gateway-lorawan-mac.cc | 8 +++---- model/lorawan-mac.cc | 4 ++-- model/lorawan-mac.h | 10 ++++----- 6 files changed, 40 insertions(+), 40 deletions(-) diff --git a/helper/lorawan-mac-helper.cc b/helper/lorawan-mac-helper.cc index 4e18b7dedb..4dbdf57d4e 100644 --- a/helper/lorawan-mac-helper.cc +++ b/helper/lorawan-mac-helper.cc @@ -200,14 +200,14 @@ LorawanMacHelper::ApplyCommonAlohaConfigurations(Ptr lorawanMac) con // SubBands // ////////////// - LogicalLoraChannelHelper channelHelper; - channelHelper.AddSubBand(868, 868.6, 1, 14); + Ptr channelHelper = CreateObject(); + channelHelper->AddSubBand(868, 868.6, 1, 14); ////////////////////// // Default channels // ////////////////////// Ptr lc1 = CreateObject(868.1, 0, 5); - channelHelper.AddChannel(lc1); + channelHelper->AddChannel(lc1); lorawanMac->SetLogicalLoraChannelHelper(channelHelper); @@ -306,10 +306,10 @@ LorawanMacHelper::ApplyCommonEuConfigurations(Ptr lorawanMac) const // SubBands // ////////////// - LogicalLoraChannelHelper channelHelper; - channelHelper.AddSubBand(868, 868.6, 0.01, 14); - channelHelper.AddSubBand(868.7, 869.2, 0.001, 14); - channelHelper.AddSubBand(869.4, 869.65, 0.1, 27); + Ptr channelHelper = CreateObject(); + channelHelper->AddSubBand(868, 868.6, 0.01, 14); + channelHelper->AddSubBand(868.7, 869.2, 0.001, 14); + channelHelper->AddSubBand(869.4, 869.65, 0.1, 27); ////////////////////// // Default channels // @@ -317,9 +317,9 @@ LorawanMacHelper::ApplyCommonEuConfigurations(Ptr lorawanMac) const Ptr lc1 = CreateObject(868.1, 0, 5); Ptr lc2 = CreateObject(868.3, 0, 5); Ptr lc3 = CreateObject(868.5, 0, 5); - channelHelper.AddChannel(lc1); - channelHelper.AddChannel(lc2); - channelHelper.AddChannel(lc3); + channelHelper->AddChannel(lc1); + channelHelper->AddChannel(lc2); + channelHelper->AddChannel(lc3); lorawanMac->SetLogicalLoraChannelHelper(channelHelper); @@ -418,16 +418,16 @@ LorawanMacHelper::ApplyCommonSingleChannelConfigurations(Ptr lorawan // SubBands // ////////////// - LogicalLoraChannelHelper channelHelper; - channelHelper.AddSubBand(868, 868.6, 0.01, 14); - channelHelper.AddSubBand(868.7, 869.2, 0.001, 14); - channelHelper.AddSubBand(869.4, 869.65, 0.1, 27); + Ptr channelHelper = CreateObject(); + channelHelper->AddSubBand(868, 868.6, 0.01, 14); + channelHelper->AddSubBand(868.7, 869.2, 0.001, 14); + channelHelper->AddSubBand(869.4, 869.65, 0.1, 27); ////////////////////// // Default channels // ////////////////////// Ptr lc1 = CreateObject(868.1, 0, 5); - channelHelper.AddChannel(lc1); + channelHelper->AddChannel(lc1); lorawanMac->SetLogicalLoraChannelHelper(channelHelper); diff --git a/model/class-a-end-device-lorawan-mac.cc b/model/class-a-end-device-lorawan-mac.cc index f59832151b..e85c9db4c8 100644 --- a/model/class-a-end-device-lorawan-mac.cc +++ b/model/class-a-end-device-lorawan-mac.cc @@ -107,7 +107,7 @@ ClassAEndDeviceLorawanMac::SendToPhy(Ptr packetToSend) Time duration = LoraPhy::GetOnAirTime(packetToSend, params); // Register the sent packet into the DutyCycleHelper - m_channelHelper.AddEvent(duration, txChannel); + m_channelHelper->AddEvent(duration, txChannel); ////////////////////////////// // Prepare for the downlink // diff --git a/model/end-device-lorawan-mac.cc b/model/end-device-lorawan-mac.cc index b5eb1cd8d0..449730ac1f 100644 --- a/model/end-device-lorawan-mac.cc +++ b/model/end-device-lorawan-mac.cc @@ -175,7 +175,7 @@ EndDeviceLorawanMac::Send(Ptr packet) // retransmissions { // Make sure we can transmit at the current power on this channel - NS_ASSERT_MSG(m_txPower <= m_channelHelper.GetTxPowerForChannel(txChannel), + NS_ASSERT_MSG(m_txPower <= m_channelHelper->GetTxPowerForChannel(txChannel), " The selected power is too high to be supported by this channel."); DoSend(packet); } @@ -513,7 +513,7 @@ EndDeviceLorawanMac::GetNextTransmissionDelay() // Pick a random channel to transmit on std::vector> logicalChannels; logicalChannels = - m_channelHelper.GetEnabledChannelList(); // Use a separate list to do the shuffle + m_channelHelper->GetEnabledChannelList(); // Use a separate list to do the shuffle // logicalChannels = Shuffle (logicalChannels); Time waitingTime = Time::Max(); @@ -526,7 +526,7 @@ EndDeviceLorawanMac::GetNextTransmissionDelay() Ptr logicalChannel = *it; double frequency = logicalChannel->GetFrequency(); - waitingTime = std::min(waitingTime, m_channelHelper.GetWaitingTime(logicalChannel)); + waitingTime = std::min(waitingTime, m_channelHelper->GetWaitingTime(logicalChannel)); NS_LOG_DEBUG("Waiting time before the next transmission in channel with frequency " << frequency << " is = " << waitingTime.GetSeconds() << "."); @@ -545,7 +545,7 @@ EndDeviceLorawanMac::GetChannelForTx() // Pick a random channel to transmit on std::vector> logicalChannels; logicalChannels = - m_channelHelper.GetEnabledChannelList(); // Use a separate list to do the shuffle + m_channelHelper->GetEnabledChannelList(); // Use a separate list to do the shuffle logicalChannels = Shuffle(logicalChannels); // Try every channel @@ -559,7 +559,7 @@ EndDeviceLorawanMac::GetChannelForTx() NS_LOG_DEBUG("Frequency of the current channel: " << frequency); // Verify that we can send the packet - Time waitingTime = m_channelHelper.GetWaitingTime(logicalChannel); + Time waitingTime = m_channelHelper->GetWaitingTime(logicalChannel); NS_LOG_DEBUG("Waiting time for current channel = " << waitingTime.GetSeconds()); @@ -696,7 +696,7 @@ EndDeviceLorawanMac::OnLinkAdrReq(uint8_t dataRate, // Check the channel mask ///////////////////////// // Check whether all specified channels exist on this device - auto channelList = m_channelHelper.GetChannelList(); + auto channelList = m_channelHelper->GetChannelList(); int channelListSize = channelList.size(); for (auto it = enabledChannels.begin(); it != enabledChannels.end(); it++) @@ -765,17 +765,17 @@ EndDeviceLorawanMac::OnLinkAdrReq(uint8_t dataRate, if (channelMaskOk && dataRateOk && txPowerOk) { // Cycle over all channels in the list - for (uint32_t i = 0; i < m_channelHelper.GetChannelList().size(); i++) + for (uint32_t i = 0; i < m_channelHelper->GetChannelList().size(); i++) { if (std::find(enabledChannels.begin(), enabledChannels.end(), i) != enabledChannels.end()) { - m_channelHelper.GetChannelList().at(i)->SetEnabledForUplink(); + m_channelHelper->GetChannelList().at(i)->SetEnabledForUplink(); NS_LOG_DEBUG("Channel " << i << " enabled"); } else { - m_channelHelper.GetChannelList().at(i)->DisableForUplink(); + m_channelHelper->GetChannelList().at(i)->DisableForUplink(); NS_LOG_DEBUG("Channel " << i << " disabled"); } } @@ -860,7 +860,7 @@ EndDeviceLorawanMac::AddLogicalChannel(double frequency) { NS_LOG_FUNCTION(this << frequency); - m_channelHelper.AddChannel(frequency); + m_channelHelper->AddChannel(frequency); } void @@ -868,7 +868,7 @@ EndDeviceLorawanMac::AddLogicalChannel(Ptr logicalChannel) { NS_LOG_FUNCTION(this << logicalChannel); - m_channelHelper.AddChannel(logicalChannel); + m_channelHelper->AddChannel(logicalChannel); } void @@ -880,7 +880,7 @@ EndDeviceLorawanMac::SetLogicalChannel(uint8_t chIndex, NS_LOG_FUNCTION(this << unsigned(chIndex) << frequency << unsigned(minDataRate) << unsigned(maxDataRate)); - m_channelHelper.SetChannel( + m_channelHelper->SetChannel( chIndex, CreateObject(frequency, minDataRate, maxDataRate)); } @@ -893,7 +893,7 @@ EndDeviceLorawanMac::AddSubBand(double startFrequency, { NS_LOG_FUNCTION_NOARGS(); - m_channelHelper.AddSubBand(startFrequency, endFrequency, dutyCycle, maxTxPowerDbm); + m_channelHelper->AddSubBand(startFrequency, endFrequency, dutyCycle, maxTxPowerDbm); } double diff --git a/model/gateway-lorawan-mac.cc b/model/gateway-lorawan-mac.cc index 9754c5afca..7dae8e43bc 100644 --- a/model/gateway-lorawan-mac.cc +++ b/model/gateway-lorawan-mac.cc @@ -60,7 +60,7 @@ GatewayLorawanMac::Send(Ptr packet) packet->AddPacketTag(tag); // Make sure we can transmit this packet - if (m_channelHelper.GetWaitingTime(CreateObject(frequency)) > Time(0)) + if (m_channelHelper->GetWaitingTime(CreateObject(frequency)) > Time(0)) { // We cannot send now! NS_LOG_WARN("Trying to send a packet but Duty Cycle won't allow it. Aborting."); @@ -83,10 +83,10 @@ GatewayLorawanMac::Send(Ptr packet) // Find the channel with the desired frequency double sendingPower = - m_channelHelper.GetTxPowerForChannel(CreateObject(frequency)); + m_channelHelper->GetTxPowerForChannel(CreateObject(frequency)); // Add the event to the channelHelper to keep track of duty cycle - m_channelHelper.AddEvent(duration, CreateObject(frequency)); + m_channelHelper->AddEvent(duration, CreateObject(frequency)); // Send the packet to the PHY layer to send it on the channel m_phy->Send(packet, params, frequency, sendingPower); @@ -143,7 +143,7 @@ GatewayLorawanMac::GetWaitingTime(double frequency) { NS_LOG_FUNCTION_NOARGS(); - return m_channelHelper.GetWaitingTime(CreateObject(frequency)); + return m_channelHelper->GetWaitingTime(CreateObject(frequency)); } } // namespace lorawan } // namespace ns3 diff --git a/model/lorawan-mac.cc b/model/lorawan-mac.cc index 0fa7323cf9..829f6aa1b2 100644 --- a/model/lorawan-mac.cc +++ b/model/lorawan-mac.cc @@ -84,14 +84,14 @@ LorawanMac::SetPhy(Ptr phy) m_phy->SetTxFinishedCallback(MakeCallback(&LorawanMac::TxFinished, this)); } -LogicalLoraChannelHelper +Ptr LorawanMac::GetLogicalLoraChannelHelper() { return m_channelHelper; } void -LorawanMac::SetLogicalLoraChannelHelper(LogicalLoraChannelHelper helper) +LorawanMac::SetLogicalLoraChannelHelper(Ptr helper) { m_channelHelper = helper; } diff --git a/model/lorawan-mac.h b/model/lorawan-mac.h index 311c57cbe9..6cb51abe6e 100644 --- a/model/lorawan-mac.h +++ b/model/lorawan-mac.h @@ -112,16 +112,16 @@ class LorawanMac : public Object /** * Get the logical lora channel helper associated with this MAC. * - * \return The instance of LogicalLoraChannelHelper that this MAC is using. + * \return A Ptr to the instance of LogicalLoraChannelHelper that this MAC is using. */ - LogicalLoraChannelHelper GetLogicalLoraChannelHelper(); + Ptr GetLogicalLoraChannelHelper(); /** * Set the LogicalLoraChannelHelper this MAC instance will use. * - * \param helper The instance of the helper to use. + * \param helper A Ptr to the instance of the helper to use. */ - void SetLogicalLoraChannelHelper(LogicalLoraChannelHelper helper); + void SetLogicalLoraChannelHelper(Ptr helper); /** * Get the spreading factor corresponding to a data rate, based on this MAC's region. @@ -243,7 +243,7 @@ class LorawanMac : public Object /** * The LogicalLoraChannelHelper instance that is assigned to this MAC. */ - LogicalLoraChannelHelper m_channelHelper; + Ptr m_channelHelper; /** * A vector holding the spreading factor each data rate corresponds to.