Possible memory issue when registering custom protocols

[giovanninardini]

Hello!

I have encountered a problem when defining a custom inet::Protocol (within a new project referencing INET) and registering it to IpProtocolGroup via the ProtocolGroup::addProtocol() method. A memory corruption occurs when executing multiple simulation runs using the same process, or when rebuilding the network from GUI (which effectively starts another simulation using the same process).

Instructions to reproduce the issue

Define a .h file including a CustomProtocol class defining the new protocol, namely NewProtocol:

class CustomProtocol {
public:
   static const inet::Protocol* newProtocol;
};
const inet::Protocol* CustomProtocol::newProtocol = new inet::Protocol("newprotocol", "NEWPROT");

Then, in another module (e.g., in its initialize() function), register it to IpProtocolGroup:

void MyModule::initialize() {
    inet::ProtocolGroup::getIpProtocolGroup()->addProtocol(CustomProtocol::newProtocol->getId(), CustomProtocol::newProtocol);
}

When the simulation runs once and the corresponding process terminates, no errors occur as expected. However, when running the simulation using Qtenv and rebuilding the network multiple times (without closing the Qtenv window, thus reusing the same process), the simulation crashes due to a double free.

Analysis

This appears to happen because the ProtocolGroup::addProtocol() method pushes the provided pointer into the dynamicallyAddedProtocols vector. When the simulation ends, the destructor of ProtocolGroup class deletes the objects referenced by this vector. Since the new protocol was defined as static, rebuilding the network does not reallocate the object. Therefore, when the simulation ends again, the destructor of ProtocolGroup attempts to delete an invalid pointer.

Possible fix

An obvious fix would be to avoid declaring the protocol as static. However, in this case I would need to instantiate an object of CustomProtocol class, which I would prefer not to do, since this class is intended only to provide custom protocol definitions.

So, I was wondering whether a viable solution would be to avoid deleting the pointers in the ProtocolGroup destructor (as a consequence, the dynamicallyAddedProtocols vector would be no longer needed and could be removed). This would fix the issue when the new protocol is defined statically, as its heap memory would be freed by the OS when the process terminates. Of course, this carries the risk of memory leaks when the new protocol is not defined as static. However, responsibility for deleting the pointer could be left to the programmer who allocated it in the first place, which seems semantically more appropriate.

Sorry for the long message!

Best regards Giovanni

[levy]

The dynamicallyAddedProtocols field has been already removed in the patch 11b7d9dc30c705054055363e44df418e5967ec0b for the same reason you are describing here. The ProtocolGroup destructor no longer deletes protocols. This patch is already in the master branch. Please check if it fixes the problem for you.

[giovanninardini]

Hi Levente,

I am sorry, I was using INET 4.5.4 and forgot to check the master branch before writing... yes, that fixes the problem! Thank you!

Best regards Giovanni

[levy]

No problem!