Cpp98 API Memory Leak

[afisher1]

Describe the bug
I have created a HELICS federate using the cpp98 api. I configured the federate to consist of two endpoints where it sends a fixed payload from one endpoint to the other. At each timestep I check for receipt of any message process said message then attempt to clear message from memory. then I create and send a new message through the designated sending endpoint then clear the newly sent message from memory. I run the simulation for 86400 seconds. I monitor my memory usage and see it steadily rise to around 2 GB for the duration of the simulation. the size of the message payload around 13 KB in size.

When doing the exact same thing using a Python HELICS federate I see no visible increase in my memory usage as  it runs.

What is the expected behavior?
I expect the memory usage to behave similar to the Python HELICS federate usage as they both use the C api under the hood.

To Reproduce
Steps to reproduce the behavior:

I'm providing a tarball with my cpp89 federate executable (helicsCppFederate) and source code is helicsCppFederate.cpp and my python federate code found in the pythonFederate folder.

running the cpp89 federate:
./helicsCppFederate federateEndpointConfig.json 86400.

running the python federate:
python helicsFederate.py federateEndpointConfig.json 86400.

both federates start up their own broker and connect to it so no need to run a broker in a separate terminal.

Environment (please complete the following information):
WSL2 Ubuntu 24.04
HELICS 3.6.1
cmake 3.28
gcc/g++ 13.3.0 

helicsCpp98Federate.tar.gz

[phlptp]

ok, got this running, Did verify the memory issue.
The problem is the msg.release() operation. That operation doesn't free the memory, it is like a shared pointer in that the release operation tells HELICS that the user assumes full responsibility for the memory, and so HELICS won't really manage any more. So on clean up in C++ it freed the wrapper but not the underlying data. The solution is to remove the msg.clear(), msg.release() lines from both the input and output operations and just let HELICS manage the memory completely. I did this in an example and ran it for a couple hours and memory usage was flat.

[phlptp]

I will add some additional documentation and warning on that operation to make it clear what that is doing. The main purpose of release is when the code needs to run it through the underlying C library and not have lifetime issues. Should maybe add a conditional compiler warning to not ignore the output of the release operation.

In your case clear() was freeing the data memory so the msg can be reused, which if you hadn't done the memory usage would have grown significantly faster. Then the release disconnected the C object from the C++ management, and then the C++ object was destroyed when it went out of scope. but no user code kept record of the C object. which just happily lived on in the buffers in the C library until the federation ended, then it would have been freed.

[trevorhardy]

We're seeing large memory usage in one use case that is using pub-sub; I assume that is a separate issue. Have you tried verifying the memory performance for pub-sub @afisher1?

[afisher1]

I provided an analogous federatePubSubConfig.json but coudn't see a similar rise in memory usage there. Need to see if I see it in the gridlabd federate for the same scenario.

[phlptp]

I did try it with the pubsubConfig as well but I didn't see any issues in that situation. I will poke some more but I suspect if there is something there it is a different issue.

[afisher1]

@phlptp so what about a case were there are more than one message in an endpoint? Should I be using a helicscpp::Message *msg in my processInputMessage routine thus making msg->clear() something I should call after pulling out the data from the message? Sounds like there would be very few scenarios where release() would need to be called as well.

[phlptp]

helicscpp::Message is a very thin wrapper around the pointer for the C message object. The C++ wrapper handles the creation and deletion of the message. So you really shouldn't use the pointers to the C++98 object. The memory for the contents will get cleared with the message, so unless you need to keep the message object around and want to reuse it clear probably isn't needed. It isn't that big of deal to have more than one message, they get reused internally when appropriate as well.

[afisher1]

I guess I am trying to look at it from a pure user's perspective with no knowledge of the underlying C api. All I know if ep.getMessage() returns an instance of helicscpp::Message and knowing I don't have the convenient use of smart pointers. I guess the proper thing to do myself would be to call free(msg) in that case. So I get confused because when I see clear, release, and free methods in the api I think oh I need to use those for proper memory handling in my code. But what you are saying is internally it's using those methods already in getMessage() and the like?

I did notice that if I tried to used any combination of helicsMessageClear()/helicsMessageRelease() methods in my python federate My federate would run into message object invalid errors when trying to assign msg a new Message Object at inconsistent iterations. commenting them out caused things to run smoothly and that should've been a clue. On a side note python's helicsMessageClear() method is missing an internal use of HelicsError in the underlying C function call.