Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct cleanup on repeated waiting for never arriving messages #43

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

ThomasDebrunner
Copy link
Contributor

@ThomasDebrunner ThomasDebrunner commented Feb 29, 2024

When calling expect(..) a shared_ptr to a std::promise is returned.
Thie shared pointer is being held by both the mav::Connection as well as the caller. The state is being cleared in the connection, iff 1) a message arrives that fulfils the expectations or 2) there is an exception on the network.

Now, if the caller excepts something that will never occur, the expectation is being held onto by the connection forever, even when the caller has long forgotten about it.
The typical case is that the caller has received a timed out while waiting for, or the expectations has gone out of scope for some other reason. In this case, this could be a resource leak.

This PR changes the internal accounting in the connection to a std::weak_ptr. If any message is received, and the handler stumbles upon an expired weak pointer, it is cleared from the callback map.

This way we make sure that we only hold references to promises that the caller is still keeping track of.

… the connection. This makes sure that promises get cleared, if they are no longer being held onto by the caller
@ThomasDebrunner ThomasDebrunner changed the title WIP: Correct cleanup on repeated waiting for never arriving messages Correct cleanup on repeated waiting for never arriving messages Feb 29, 2024
@ThomasDebrunner ThomasDebrunner marked this pull request as ready for review February 29, 2024 14:30
@eyeam3
Copy link

eyeam3 commented Feb 29, 2024

I had the test app running for about 10 minutes and the CPU load stayed constant.

@ThomasDebrunner ThomasDebrunner merged commit 67b819b into main Feb 29, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants