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

Fix a memory leak that occurs with keep alive #155

Merged
merged 2 commits into from
Dec 8, 2016
Merged

Conversation

shmuelk
Copy link
Collaborator

@shmuelk shmuelk commented Dec 7, 2016

Fix a memory leak that occurs when sockets kept open due to keep alive are finally closed due to inactivity.

Description

When sockets are closed due to inactivity make sure that the IncomingSocketProcessor.prepareToClose() function is called. This will break the retain loop between the HTTPIncomingMesage and the HTTPParser.

Motivation and Context

Fixes a memory lak reported in isuue Kitura/Kitura#902

How Has This Been Tested?

@djones tested using Safari and determined that the memory leak was fixed.
Ran KituraNet unit tests on macOS and Linux.

Checklist:

  • I have submitted a CLA form
  • If applicable, I have updated the documentation accordingly.
  • If applicable, I have added tests to cover my changes.

@djones6
Copy link
Contributor

djones6 commented Dec 7, 2016

My method for testing this was to refresh the project that @carlbrown provided for Kitura/Kitura#902 on Mac using Safari (which seems, at least for me, to sometimes ditch existing connections and open new ones). The leak seems to be gone, though my method of testing wasn't very scientific. I no longer see an accumulation of instances of ServerRequest / HTTPParser in the xcode debugger.

I'll work on a more repeatable benchmark as we need regression testing for this. This case is non-trivial as it needs to open connections, drive some requests, then let them idle so that we can witness the memory usage reduce once another incoming connection kicks off our idleness check.

@ianpartridge ianpartridge merged commit 93ddb41 into master Dec 8, 2016
@ianpartridge ianpartridge deleted the issue_902 branch December 8, 2016 10:48
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.

3 participants