Skip to content

Commit

Permalink
Transports/CommManager: fix several serious and continuous leaks
Browse files Browse the repository at this point in the history
Messages were being cloned (new) without being properly released.
- Avoided cloning where possible
- Release resources
- Make sure order of deleting pointers and erasing elements from
  container is correct
  • Loading branch information
José Braga committed Apr 29, 2022
1 parent 4f2a7d1 commit 9d8bdbf
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 70 deletions.
66 changes: 31 additions & 35 deletions src/Transports/CommManager/Router.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,30 +51,29 @@ namespace Transports
{

public:
Router(Task* task)
{
m_parent = task;
m_medium = 4;
m_gsm_entity_id = -1;
m_iridium_entity_id = -1;
m_reqid = 0;
c_wifi_timeout = 15;
}
explicit Router(Task* const task):
m_parent(task),
m_medium(4),
m_gsm_entity_id(-1),
m_iridium_entity_id(-1),
m_reqid(0),
c_wifi_timeout(15)
{ }

void
process(IMC::VehicleMedium* msg)
process(const IMC::VehicleMedium* msg)
{
m_medium = msg->medium;
}

void
process(IMC::RSSI* msg)
process(const IMC::RSSI* msg)
{
m_rssi_msg_map[msg->getSourceEntity()] = msg->value;
}

void
process(IMC::Announce* msg)
process(const IMC::Announce* msg)
{
std::vector<std::string> list;
String::split(msg->services, ";", list);
Expand Down Expand Up @@ -172,19 +171,18 @@ namespace Transports
void
clearTimeouts()
{
std::map<uint16_t, IMC::TransmissionRequest*>::iterator it;
double time = Time::Clock::getSinceEpoch();
it = m_transmission_requests.begin();
auto it = m_transmission_requests.begin();

while (it != m_transmission_requests.end())
{
if (it->second->deadline <= time)
{
m_parent->inf("Transmission Request %d is expired by %f seconds", it->second->req_id, it->second->deadline - time);
m_parent->inf("Transmission Request %d is expired by %f seconds", it->second->req_id, it->second->deadline - time);
answer(it->second, "Transmission timed out.",
IMC::TransmissionStatus::TSTAT_TEMPORARY_FAILURE);
Memory::clear(it->second);
m_transmission_requests.erase(it++);
m_transmission_requests.erase(it);
}
else
++it;
Expand Down Expand Up @@ -214,7 +212,7 @@ namespace Transports
case IMC::TransmissionRequest::DMODE_INLINEMSG:
{
tx.type = IMC::AcousticRequest::TYPE_MSG;
tx.msg.set(msg->msg_data.get()->clone());
tx.msg.set(*msg->msg_data.get());

break;
}
Expand Down Expand Up @@ -291,10 +289,9 @@ namespace Transports
case IMC::TransmissionRequest::DMODE_INLINEMSG:
{
IMC::ImcIridiumMessage m;
const IMC::Message * inlinemsg = msg->msg_data.get();
m.destination = 0xFFFF;
m.source = m_parent->getSystemId();
m.msg = inlinemsg->clone();
m.msg = msg->msg_data.get()->clone();
uint8_t buffer[65535];
int len = m.serialize(buffer);
tx.data.assign(buffer, buffer + len);
Expand Down Expand Up @@ -578,9 +575,9 @@ namespace Transports
{
uint16_t newId = createInternalId();
m_transmission_requests[newId] = msg->clone();
IMC::TransmissionRequest* msg2 = msg->clone();
msg2->req_id = newId;
answerTCPStatus(msg2, "Didn't find TCP server on destination host",
IMC::TransmissionRequest msg2 = *msg;
msg2.req_id = newId;
answerTCPStatus(&msg2, "Didn't find TCP server on destination host",
IMC::TCPStatus::TCPSTAT_HOST_UNKNOWN);
return;
}
Expand All @@ -592,7 +589,7 @@ namespace Transports
{
case IMC::TransmissionRequest::DMODE_INLINEMSG:
{
send.msg_data.set(msg->msg_data.get()->clone());
send.msg_data.set(*msg->msg_data.get());
break;
}
default:
Expand Down Expand Up @@ -747,18 +744,20 @@ namespace Transports
m_iridium_entity_id = id;
}

std::map<uint16_t, IMC::TransmissionRequest*>*
std::map<uint16_t, IMC::TransmissionRequest*>&
getList()
{
return &m_transmission_requests;
return m_transmission_requests;
}

~Router()
{
for (auto& pair : m_transmission_requests)
Memory::clear(pair.second);
}

private:
Task* m_parent;
Task* const m_parent;
uint8_t m_medium;

int m_gsm_entity_id;
Expand Down Expand Up @@ -809,9 +808,7 @@ namespace Transports
if (system.empty())
return false;

std::map<std::string, TCPAnnounce>::iterator it;

it = m_wifi_map.find(system);
auto it = m_wifi_map.find(system);
if (it == m_wifi_map.end())
return false;

Expand All @@ -828,8 +825,7 @@ namespace Transports
if (system.empty())
return false;

std::map<std::string, std::string>::iterator it;
it = m_gsm_announce_map.find(system);
auto it = m_gsm_announce_map.find(system);
if (it != m_gsm_announce_map.end())
{
std = it->second;
Expand All @@ -855,8 +851,8 @@ namespace Transports
{
if (m_gsm_entity_id == -1)
return false;
std::map<uint8_t, fp32_t>::iterator it;
it = m_rssi_msg_map.find(m_gsm_entity_id);

auto it = m_rssi_msg_map.find(m_gsm_entity_id);
if (it == m_rssi_msg_map.end())
return false;
if (it->second > 0)
Expand All @@ -869,8 +865,8 @@ namespace Transports
{
if (m_iridium_entity_id == -1)
return false;
std::map<uint8_t, fp32_t>::iterator it;
it = m_rssi_msg_map.find(m_iridium_entity_id);

auto it = m_rssi_msg_map.find(m_iridium_entity_id);
if (it == m_rssi_msg_map.end())
return false;
if (it->second >= 20)
Expand Down
Loading

0 comments on commit 9d8bdbf

Please sign in to comment.