Skip to content

Commit

Permalink
[MAC] Fix LogicalLoraChannelHelper Object double destruction (#180)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@non-det-alle
non-det-alle authored Nov 5, 2024
1 parent 82ea58a commit c1bd68b
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 40 deletions.
30 changes: 15 additions & 15 deletions helper/lorawan-mac-helper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,14 +200,14 @@ LorawanMacHelper::ApplyCommonAlohaConfigurations(Ptr<LorawanMac> lorawanMac) con
// SubBands //
//////////////

LogicalLoraChannelHelper channelHelper;
channelHelper.AddSubBand(868, 868.6, 1, 14);
Ptr<LogicalLoraChannelHelper> channelHelper = CreateObject<LogicalLoraChannelHelper>();
channelHelper->AddSubBand(868, 868.6, 1, 14);

//////////////////////
// Default channels //
//////////////////////
Ptr<LogicalLoraChannel> lc1 = CreateObject<LogicalLoraChannel>(868.1, 0, 5);
channelHelper.AddChannel(lc1);
channelHelper->AddChannel(lc1);

lorawanMac->SetLogicalLoraChannelHelper(channelHelper);

Expand Down Expand Up @@ -306,20 +306,20 @@ LorawanMacHelper::ApplyCommonEuConfigurations(Ptr<LorawanMac> 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<LogicalLoraChannelHelper> channelHelper = CreateObject<LogicalLoraChannelHelper>();
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<LogicalLoraChannel> lc1 = CreateObject<LogicalLoraChannel>(868.1, 0, 5);
Ptr<LogicalLoraChannel> lc2 = CreateObject<LogicalLoraChannel>(868.3, 0, 5);
Ptr<LogicalLoraChannel> lc3 = CreateObject<LogicalLoraChannel>(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);

Expand Down Expand Up @@ -418,16 +418,16 @@ LorawanMacHelper::ApplyCommonSingleChannelConfigurations(Ptr<LorawanMac> 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<LogicalLoraChannelHelper> channelHelper = CreateObject<LogicalLoraChannelHelper>();
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<LogicalLoraChannel> lc1 = CreateObject<LogicalLoraChannel>(868.1, 0, 5);
channelHelper.AddChannel(lc1);
channelHelper->AddChannel(lc1);

lorawanMac->SetLogicalLoraChannelHelper(channelHelper);

Expand Down
2 changes: 1 addition & 1 deletion model/class-a-end-device-lorawan-mac.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ ClassAEndDeviceLorawanMac::SendToPhy(Ptr<Packet> 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 //
Expand Down
26 changes: 13 additions & 13 deletions model/end-device-lorawan-mac.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ EndDeviceLorawanMac::Send(Ptr<Packet> 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);
}
Expand Down Expand Up @@ -513,7 +513,7 @@ EndDeviceLorawanMac::GetNextTransmissionDelay()
// Pick a random channel to transmit on
std::vector<Ptr<LogicalLoraChannel>> 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();
Expand All @@ -526,7 +526,7 @@ EndDeviceLorawanMac::GetNextTransmissionDelay()
Ptr<LogicalLoraChannel> 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() << ".");
Expand All @@ -545,7 +545,7 @@ EndDeviceLorawanMac::GetChannelForTx()
// Pick a random channel to transmit on
std::vector<Ptr<LogicalLoraChannel>> 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
Expand All @@ -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());

Expand Down Expand Up @@ -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++)
Expand Down Expand Up @@ -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");
}
}
Expand Down Expand Up @@ -860,15 +860,15 @@ EndDeviceLorawanMac::AddLogicalChannel(double frequency)
{
NS_LOG_FUNCTION(this << frequency);

m_channelHelper.AddChannel(frequency);
m_channelHelper->AddChannel(frequency);
}

void
EndDeviceLorawanMac::AddLogicalChannel(Ptr<LogicalLoraChannel> logicalChannel)
{
NS_LOG_FUNCTION(this << logicalChannel);

m_channelHelper.AddChannel(logicalChannel);
m_channelHelper->AddChannel(logicalChannel);
}

void
Expand All @@ -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<LogicalLoraChannel>(frequency, minDataRate, maxDataRate));
}
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions model/gateway-lorawan-mac.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ GatewayLorawanMac::Send(Ptr<Packet> packet)
packet->AddPacketTag(tag);

// Make sure we can transmit this packet
if (m_channelHelper.GetWaitingTime(CreateObject<LogicalLoraChannel>(frequency)) > Time(0))
if (m_channelHelper->GetWaitingTime(CreateObject<LogicalLoraChannel>(frequency)) > Time(0))
{
// We cannot send now!
NS_LOG_WARN("Trying to send a packet but Duty Cycle won't allow it. Aborting.");
Expand All @@ -83,10 +83,10 @@ GatewayLorawanMac::Send(Ptr<Packet> packet)

// Find the channel with the desired frequency
double sendingPower =
m_channelHelper.GetTxPowerForChannel(CreateObject<LogicalLoraChannel>(frequency));
m_channelHelper->GetTxPowerForChannel(CreateObject<LogicalLoraChannel>(frequency));

// Add the event to the channelHelper to keep track of duty cycle
m_channelHelper.AddEvent(duration, CreateObject<LogicalLoraChannel>(frequency));
m_channelHelper->AddEvent(duration, CreateObject<LogicalLoraChannel>(frequency));

// Send the packet to the PHY layer to send it on the channel
m_phy->Send(packet, params, frequency, sendingPower);
Expand Down Expand Up @@ -143,7 +143,7 @@ GatewayLorawanMac::GetWaitingTime(double frequency)
{
NS_LOG_FUNCTION_NOARGS();

return m_channelHelper.GetWaitingTime(CreateObject<LogicalLoraChannel>(frequency));
return m_channelHelper->GetWaitingTime(CreateObject<LogicalLoraChannel>(frequency));
}
} // namespace lorawan
} // namespace ns3
4 changes: 2 additions & 2 deletions model/lorawan-mac.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,14 @@ LorawanMac::SetPhy(Ptr<LoraPhy> phy)
m_phy->SetTxFinishedCallback(MakeCallback(&LorawanMac::TxFinished, this));
}

LogicalLoraChannelHelper
Ptr<LogicalLoraChannelHelper>
LorawanMac::GetLogicalLoraChannelHelper()
{
return m_channelHelper;
}

void
LorawanMac::SetLogicalLoraChannelHelper(LogicalLoraChannelHelper helper)
LorawanMac::SetLogicalLoraChannelHelper(Ptr<LogicalLoraChannelHelper> helper)
{
m_channelHelper = helper;
}
Expand Down
10 changes: 5 additions & 5 deletions model/lorawan-mac.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<LogicalLoraChannelHelper> 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<LogicalLoraChannelHelper> helper);

/**
* Get the spreading factor corresponding to a data rate, based on this MAC's region.
Expand Down Expand Up @@ -243,7 +243,7 @@ class LorawanMac : public Object
/**
* The LogicalLoraChannelHelper instance that is assigned to this MAC.
*/
LogicalLoraChannelHelper m_channelHelper;
Ptr<LogicalLoraChannelHelper> m_channelHelper;

/**
* A vector holding the spreading factor each data rate corresponds to.
Expand Down

0 comments on commit c1bd68b

Please sign in to comment.