{"action":"created","issue":{"url":"https://api.github.com/repos/Azure/WALinuxAgent/issues/1154","repository_url":"https://api.github.com/repos/Azure/WALinuxAgent","labels_url":"https://api.github.com/repos/Azure/WALinuxAgent/issues/1154/labels{/name}","comments_url":"https://api.github.com/repos/Azure/WALinuxAgent/issues/1154/comments","events_url":"https://api.github.com/repos/Azure/WALinuxAgent/issues/1154/events","html_url":"https://github.com/Azure/WALinuxAgent/pull/1154","id":321251863,"number":1154,"title":"set the Connection close header","user":{"login":"boumenot","id":91112,"avatar_url":"https://avatars2.githubusercontent.com/u/91112?v=4","gravatar_id":"","url":"https://api.github.com/users/boumenot","html_url":"https://github.com/boumenot","followers_url":"https://api.github.com/users/boumenot/followers","following_url":"https://api.github.com/users/boumenot/following{/other_user}","gists_url":"https://api.github.com/users/boumenot/gists{/gist_id}","starred_url":"https://api.github.com/users/boumenot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/boumenot/subscriptions","organizations_url":"https://api.github.com/users/boumenot/orgs","repos_url":"https://api.github.com/users/boumenot/repos","events_url":"https://api.github.com/users/boumenot/events{/privacy}","received_events_url":"https://api.github.com/users/boumenot/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-05-08T16:11:08Z","updated_at":"2018-05-08T21:45:57Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/Azure/WALinuxAgent/pulls/1154","html_url":"https://github.com/Azure/WALinuxAgent/pull/1154","diff_url":"https://github.com/Azure/WALinuxAgent/pull/1154.diff","patch_url":"https://github.com/Azure/WALinuxAgent/pull/1154.patch"},"body":"There is a slow-ish memory leak in the agent's HTTP request library.  The leak has been confirmed on OpenSuSE and Debian thus far. I have not been able to reproduce it on Ubuntu, RHEL, or CentOS.\r\n\r\nI created a [branch](https://github.com/Azure/WALinuxAgent/tree/pr-tracemalloc) with tracemalloc instrumentation to help track down the issue. The leak appeared with the following file and line context.\r\n\r\n```text\r\nTop 50 lines\r\n#1: python3.5/socket.py:647: 13609.4 KiB\r\n    self._sock = None\r\n```\r\n\r\nThis was just useful enough to identify that the issue had to do with sockets.  (tracemalloc supports grabbing a larger stacktrace around the memory allocation source, but I was not able to get that additional context.  I have not debugged why.)  The couple of bare socket calls that the agent makes are infrequent. The agent does make a lot of HTTP requests, so I started with the HTTP library.\r\n\r\nThe first thing I noticed is that the agent never explicitly cleans up an HTTP connection when it is done with it.  Note that the following method never calls `resp.connection.close()`.\r\n\r\n```python\r\n    def put_block_blob(self, url, data):\r\n        logger.verbose(\"Put block blob\")\r\n        headers = self.get_block_blob_headers(len(data))\r\n        resp = self.client.call_storage_service(restutil.http_put, url, data, headers)\r\n        if resp.status != httpclient.CREATED:\r\n            raise UploadError(\r\n                \"Failed to upload block blob: {0}\".format(resp.status))\r\n```\r\n\r\nI thought this was fine. I would expect Python to clean up once all references were removed, but this does not appear to be true at least in the case of Debian/OpenSuSE.  I could fix the code to explicitly close the connection, but there are many places and branches where the agent makes HTTP requests.  The HTTP library returns the HTTPResponse and not the connection, which is generally fine because you can **usually** reach the connection via the response. Another fix, that appears to be working, is to explicitly set the HTTP Connection header to close.\r\n\r\nIn my travels, I found that the Python requests library also had this issue.  Their fix was to explicitly close the connection after use.  I think that we should make that fix too, but that is a bigger and more invasive change.  I would prefer to ship this fix as a hotfix, and leave the bigger fix for another release.\r\n\r\n\r\nCloses #1092 "},"comment":{"url":"https://api.github.com/repos/Azure/WALinuxAgent/issues/comments/387552619","html_url":"https://github.com/Azure/WALinuxAgent/pull/1154#issuecomment-387552619","issue_url":"https://api.github.com/repos/Azure/WALinuxAgent/issues/1154","id":387552619,"user":{"login":"jasonzio","id":8397537,"avatar_url":"https://avatars2.githubusercontent.com/u/8397537?v=4","gravatar_id":"","url":"https://api.github.com/users/jasonzio","html_url":"https://github.com/jasonzio","followers_url":"https://api.github.com/users/jasonzio/followers","following_url":"https://api.github.com/users/jasonzio/following{/other_user}","gists_url":"https://api.github.com/users/jasonzio/gists{/gist_id}","starred_url":"https://api.github.com/users/jasonzio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasonzio/subscriptions","organizations_url":"https://api.github.com/users/jasonzio/orgs","repos_url":"https://api.github.com/users/jasonzio/repos","events_url":"https://api.github.com/users/jasonzio/events{/privacy}","received_events_url":"https://api.github.com/users/jasonzio/received_events","type":"User","site_admin":false},"created_at":"2018-05-08T21:45:57Z","updated_at":"2018-05-08T21:45:57Z","author_association":"MEMBER","body":"I think there's a close() missing in restutil.http_request (restutil.py line 324 right before the final `raise HttpError(...)` at the end of the function). If a retryable HTTP request hits the maximum number of attempts, it looks to me like the connection leaks."}}
{"actor":{"display_login":"jasonzio"}}