Fixes to prevent memory leaks in RTI #446
Conversation
WalkthroughRecent updates in the Changes
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- core/federated/RTI/rti_remote.c (2 hunks)
- core/utils/pqueue_tag.c (1 hunks)
Additional comments not posted (3)
core/utils/pqueue_tag.c (1)
150-150: Approved change inpqueue_tag_remove_up_tofunction.The replacement of
pqueue_tag_popwithpqueue_tag_pop_tagin the loop is a significant improvement, as it ensures that dynamically allocated elements are properly freed, potentially preventing memory leaks.core/federated/RTI/rti_remote.c (2)
1689-1690: Thread join operation added forresponder_thread.The addition of a thread join operation for
responder_threadensures that the RTI properly waits for this thread to complete before proceeding. This is crucial for avoiding potential race conditions or premature termination of threads which could lead to undefined behavior or resource leaks.
1752-1762: Enhanced memory deallocation infree_scheduling_nodes.The changes made to
free_scheduling_nodesfunction are critical for preventing memory leaks. The explicit deallocation ofupstream,upstream_delay,min_delays, anddownstreamarrays, as well as the node itself, ensures that all dynamically allocated memory is properly freed. This is in line with best practices for memory management in C.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- core/federated/RTI/rti_remote.c (1 hunks)
- core/utils/pqueue_tag.c (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- core/federated/RTI/rti_remote.c
- core/utils/pqueue_tag.c
lhstrh
changed the title
* Free elements when removing them from a queue * Free calloc'ed elements when terminating * Explicitly join threads
This PR resolves memory leaks from the RTI. I attached the output file from Valgrind.
valgrind-out-before.txt
With this PR, most of the leakages have disappeared.
valgrind-out-after.txt
Specifically, now the function
pqueue_tag_remove_up_tofrees dynamically allocated elements. Also, memory allocated for storing each node'supstreamandmin_delaysis explicitly freed when the RTI terminates.Valgrind reports two remaining issues. The two threads shown below are not explicitly joined.
reactor-c/core/federated/RTI/rti_remote.c
Line 666 in 587a8f9
This thread is created in the function handle_stop_request_message to set a timeout to wait for
MSG_TYPE_STOP_REQUEST_REPLYmessages. Thus, this thread cannot be joined in the functionhandle_stop_request_messageand I didn't come up with joining this function.reactor-c/core/federated/RTI/rti_remote.c
Line 1677 in 587a8f9
This thread is responsible for responding to erroneous connection requests. Thus, this thread should not be joined.
Summary by CodeRabbit
Bug Fixes
responder_threadto ensure proper synchronization.free_scheduling_nodes.Refactor