)]}'
{"swift/proxy/controllers/obj.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7b3514a523e744bfa9ab13e8def6c234c244e427","unresolved":false,"context_lines":[{"line_number":1025,"context_line":""},{"line_number":1026,"context_line":"        try:"},{"line_number":1027,"context_line":"            self.stashed_iter \u003d reiterate(self._real_iter(req, resp.headers))"},{"line_number":1028,"context_line":"        except Exception:"},{"line_number":1029,"context_line":"            self.close()"},{"line_number":1030,"context_line":"            raise"},{"line_number":1031,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_177dbf1b","line":1028,"updated":"2017-12-29 01:18:08.000000000","message":"Huh. I would\u0027ve guessed this would need a\n\n # noqa\n\nFWIW, the traceback here is\n\nTraceback (most recent call last):\n  File \".../swift/proxy/controllers/obj.py\", line 1027, in kickoff\n    self.stashed_iter \u003d reiterate(self._real_iter(req, resp.headers))\n  File \".../swift/proxy/controllers/obj.py\", line 1272, in _real_iter\n    convert_ranges_iter(), self.mime_boundary, multipart, self.logger)\n  File \".../swift/common/utils.py\", line 4102, in document_iters_to_http_response_body\n    response_body_iter \u003d next(ranges_iter)[\u0027part_iter\u0027]\n  File \".../swift/proxy/controllers/obj.py\", line 1246, in convert_ranges_iter\n    request\u003dreq, headers\u003dresp_headers)\nHTTPException: 416 Requested Range Not Satisfiable\n\n...but I\u0027m pretty sure I like the blanket Exception handling.","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"c723da55eaec1c39edbb3647bf9f7ee41820af1f","unresolved":false,"context_lines":[{"line_number":1025,"context_line":""},{"line_number":1026,"context_line":"        try:"},{"line_number":1027,"context_line":"            self.stashed_iter \u003d reiterate(self._real_iter(req, resp.headers))"},{"line_number":1028,"context_line":"        except Exception:"},{"line_number":1029,"context_line":"            self.close()"},{"line_number":1030,"context_line":"            raise"},{"line_number":1031,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_c51ebc6f","line":1028,"in_reply_to":"9f91af0f_177dbf1b","updated":"2017-12-29 18:22:23.000000000","message":"bare \"except:\" needs \"# noqa\", but as long as you specify any exception class, it\u0027s fine","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"159738fa480a36c66404c9875070d7885b4e3bc4","unresolved":false,"context_lines":[{"line_number":2352,"context_line":"            except HTTPException as err_resp:"},{"line_number":2353,"context_line":"                # catch any HTTPException response here so that we can"},{"line_number":2354,"context_line":"                # process response headers uniformly in _fix_response"},{"line_number":2355,"context_line":"                resp \u003d err_resp"},{"line_number":2356,"context_line":"        else:"},{"line_number":2357,"context_line":"            # TODO: we can get here if all buckets are successful but none"},{"line_number":2358,"context_line":"            # have ec_ndata getters, so bad_bucket may have no gets and we will"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f91af0f_d0f89cc5","line":2355,"updated":"2018-01-02 22:26:40.000000000","message":"Hmm... we never deal with the other buckets here...\n\nIf I PUT an object, shut down a primary, overwrite with another object, then bring everybody up again, I can still leak sockets as the object servers all return 2xx but the connections to the out-of-date servers never close.","commit_id":"f709eed41b9579cf5a8ca9180143301a2b452d47"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"1adb32eff9bf9540fa5e651bdff1afee5abf1766","unresolved":false,"context_lines":[{"line_number":2352,"context_line":"            except HTTPException as err_resp:"},{"line_number":2353,"context_line":"                # catch any HTTPException response here so that we can"},{"line_number":2354,"context_line":"                # process response headers uniformly in _fix_response"},{"line_number":2355,"context_line":"                resp \u003d err_resp"},{"line_number":2356,"context_line":"        else:"},{"line_number":2357,"context_line":"            # TODO: we can get here if all buckets are successful but none"},{"line_number":2358,"context_line":"            # have ec_ndata getters, so bad_bucket may have no gets and we will"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f96bb07_65cc6635","line":2355,"in_reply_to":"9f91af0f_d0f89cc5","updated":"2018-01-12 09:15:53.000000000","message":"it might be true. I didn\u0027t check it with actual running environment but backend app_iter (for object servers) in other ResponseBuckets doesn\u0027t look to be cared as closed.","commit_id":"f709eed41b9579cf5a8ca9180143301a2b452d47"}],"test/unit/proxy/test_server.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7b3514a523e744bfa9ab13e8def6c234c244e427","unresolved":false,"context_lines":[{"line_number":7173,"context_line":"        unclosed_http_responses \u003d {}"},{"line_number":7174,"context_line":"        tracked_responses \u003d [0]"},{"line_number":7175,"context_line":""},{"line_number":7176,"context_line":"        class LeakTrackingHTTPResponse(BufferedHTTPResponse):"},{"line_number":7177,"context_line":"            def begin(self):"},{"line_number":7178,"context_line":"                # no super(); we inherit from an old-style class (it\u0027s"},{"line_number":7179,"context_line":"                # httplib\u0027s fault; don\u0027t try and fix it)."}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_f7a85b78","line":7176,"updated":"2017-12-29 01:18:08.000000000","message":"Now I kinda want this in more places.","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7b3514a523e744bfa9ab13e8def6c234c244e427","unresolved":false,"context_lines":[{"line_number":7181,"context_line":"                if self.status !\u003d 204:"},{"line_number":7182,"context_line":"                    # This mock is overly broad and catches account and"},{"line_number":7183,"context_line":"                    # container HEAD requests too. We don\u0027t care about"},{"line_number":7184,"context_line":"                    # those; it\u0027s the object GETs that were leaky."},{"line_number":7185,"context_line":"                    #"},{"line_number":7186,"context_line":"                    # Unfortunately, we don\u0027t have access to the request"},{"line_number":7187,"context_line":"                    # path here, so we use \"status \u003d\u003d 204\" as a crude proxy"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_f7c13b30","line":7184,"updated":"2017-12-29 01:18:08.000000000","message":"...but they *are* leaking, yeah? I don\u0027t see any close() in get_container_info...","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"159738fa480a36c66404c9875070d7885b4e3bc4","unresolved":false,"context_lines":[{"line_number":7181,"context_line":"                if self.status !\u003d 204:"},{"line_number":7182,"context_line":"                    # This mock is overly broad and catches account and"},{"line_number":7183,"context_line":"                    # container HEAD requests too. We don\u0027t care about"},{"line_number":7184,"context_line":"                    # those; it\u0027s the object GETs that were leaky."},{"line_number":7185,"context_line":"                    #"},{"line_number":7186,"context_line":"                    # Unfortunately, we don\u0027t have access to the request"},{"line_number":7187,"context_line":"                    # path here, so we use \"status \u003d\u003d 204\" as a crude proxy"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_90e454ae","line":7184,"in_reply_to":"9f91af0f_800d0239","updated":"2018-01-02 22:26:40.000000000","message":"\u003e Search for \"will_close\" in Lib/httplib.py in the Python sources.\n\nI\u0027m still confused. Looking at https://github.com/python/cpython/blob/75b2c5f/Lib/httplib.py#L1125-L1130 (which *I think* is the relevant place?) it seems like response.will_close indicates who \"owns\" the socket -- if True, then the response object owns it and you should call close on it; otherwise the connection owns it and you should call close on *that*. But if I scribble down self.will_close for all begin() calls (204 or no), they *all* say True!","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"c2d243e3b05137cdc6ff52e63369ed6f52f9290b","unresolved":false,"context_lines":[{"line_number":7181,"context_line":"                if self.status !\u003d 204:"},{"line_number":7182,"context_line":"                    # This mock is overly broad and catches account and"},{"line_number":7183,"context_line":"                    # container HEAD requests too. We don\u0027t care about"},{"line_number":7184,"context_line":"                    # those; it\u0027s the object GETs that were leaky."},{"line_number":7185,"context_line":"                    #"},{"line_number":7186,"context_line":"                    # Unfortunately, we don\u0027t have access to the request"},{"line_number":7187,"context_line":"                    # path here, so we use \"status \u003d\u003d 204\" as a crude proxy"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_56bc84db","line":7184,"in_reply_to":"9f91af0f_90e454ae","updated":"2018-01-03 00:27:47.000000000","message":"Yeah, I\u0027m confused too. That stuff is a giant mess. You\u0027ve got these two httplib objects and you should probably close one of them, or maybe both, but sometimes we don\u0027t close either one and it works.\n\nUnderneath all that you\u0027ve got these Python socket objects hanging out that close the socket fd when their refcount drops to 0, so in CPython, you\u0027re probably okay to drop the httplib objects on the ground. The trouble happens when you have a circular reference somewhere that keeps the httplib objects alive and thereby keeps the socket alive.\n\nIt\u0027s very forgiving right up until it isn\u0027t. :|","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"c723da55eaec1c39edbb3647bf9f7ee41820af1f","unresolved":false,"context_lines":[{"line_number":7181,"context_line":"                if self.status !\u003d 204:"},{"line_number":7182,"context_line":"                    # This mock is overly broad and catches account and"},{"line_number":7183,"context_line":"                    # container HEAD requests too. We don\u0027t care about"},{"line_number":7184,"context_line":"                    # those; it\u0027s the object GETs that were leaky."},{"line_number":7185,"context_line":"                    #"},{"line_number":7186,"context_line":"                    # Unfortunately, we don\u0027t have access to the request"},{"line_number":7187,"context_line":"                    # path here, so we use \"status \u003d\u003d 204\" as a crude proxy"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_800d0239","line":7184,"in_reply_to":"9f91af0f_f7c13b30","updated":"2017-12-29 18:22:23.000000000","message":"Experimentally, no. You don\u0027t see any CLOSE_WAIT sockets hanging around after calling get_container_info.\n\nFor some reason, httplib sometimes wants you to close the HTTPResponse object and sometimes wants you to close the HTTPConnection object. Search for \"will_close\" in Lib/httplib.py in the Python sources.\n\nContainer HEAD responses don\u0027t have to be closed for some reason, and it doesn\u0027t cause any socket leak. Object GET responses have to be closed, though, as long as their status is 2xx (and their length is \u003e 0, I think).","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7b3514a523e744bfa9ab13e8def6c234c244e427","unresolved":false,"context_lines":[{"line_number":7183,"context_line":"                    # container HEAD requests too. We don\u0027t care about"},{"line_number":7184,"context_line":"                    # those; it\u0027s the object GETs that were leaky."},{"line_number":7185,"context_line":"                    #"},{"line_number":7186,"context_line":"                    # Unfortunately, we don\u0027t have access to the request"},{"line_number":7187,"context_line":"                    # path here, so we use \"status \u003d\u003d 204\" as a crude proxy"},{"line_number":7188,"context_line":"                    # for \"not an object response\"."},{"line_number":7189,"context_line":"                    unclosed_http_responses[id(self)] \u003d self"},{"line_number":7190,"context_line":"                    tracked_responses[0] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_57e6e7ea","line":7187,"range":{"start_line":7186,"start_character":37,"end_line":7187,"end_character":31},"updated":"2017-12-29 01:18:08.000000000","message":"That *is* unfortunate! Failure isn\u0027t terribly informative:\n\nFAIL: test_unsatisfiable_socket_leak (test.unit.proxy.test_server.TestObjectECRangedGET)\n----------------------------------------------------------------------\nTraceback (most recent call last):\n  File \"/vagrant/swift/test/unit/proxy/test_server.py\", line 7205, in test_unsatisfiable_socket_leak\n    self.assertEqual(unclosed_http_responses, {})\nAssertionError: {140686297314440: \u003ctest.unit.proxy.test_server.LeakTrackingHTTPResponse instance [truncated]... !\u003d {}\n+ {}\n- {140686297314440: \u003ctest.unit.proxy.test_server.LeakTrackingHTTPResponse instance at 0x7ff414c6b488\u003e,\n-  140686297316600: \u003ctest.unit.proxy.test_server.LeakTrackingHTTPResponse instance at 0x7ff414c6bcf8\u003e}\n\nAlthough... after digging around a bit, I got to\n\n            def __repr__(self):\n                return \u0027LeakTrackingHTTPResponse\u003c%s %s\u003e\u0027 % (\n                    self.swift_conn._method, self.swift_conn._path)\n\n...but maybe there\u0027s a little too much \"magic\" there.","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"c723da55eaec1c39edbb3647bf9f7ee41820af1f","unresolved":false,"context_lines":[{"line_number":7183,"context_line":"                    # container HEAD requests too. We don\u0027t care about"},{"line_number":7184,"context_line":"                    # those; it\u0027s the object GETs that were leaky."},{"line_number":7185,"context_line":"                    #"},{"line_number":7186,"context_line":"                    # Unfortunately, we don\u0027t have access to the request"},{"line_number":7187,"context_line":"                    # path here, so we use \"status \u003d\u003d 204\" as a crude proxy"},{"line_number":7188,"context_line":"                    # for \"not an object response\"."},{"line_number":7189,"context_line":"                    unclosed_http_responses[id(self)] \u003d self"},{"line_number":7190,"context_line":"                    tracked_responses[0] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_802662b7","line":7187,"range":{"start_line":7186,"start_character":37,"end_line":7187,"end_character":31},"in_reply_to":"9f91af0f_57e6e7ea","updated":"2017-12-29 18:22:23.000000000","message":"Sure, I can do that, or something pretty close.","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7b3514a523e744bfa9ab13e8def6c234c244e427","unresolved":false,"context_lines":[{"line_number":7191,"context_line":"                return retval"},{"line_number":7192,"context_line":""},{"line_number":7193,"context_line":"            def close(self, *args, **kwargs):"},{"line_number":7194,"context_line":"                rv \u003d BufferedHTTPResponse.close(self, *args, **kwargs)"},{"line_number":7195,"context_line":"                unclosed_http_responses.pop(id(self), None)"},{"line_number":7196,"context_line":"                return rv"},{"line_number":7197,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_d7e3d7a3","line":7194,"updated":"2017-12-29 01:18:08.000000000","message":"So are we worried about closing the *Response*, or the *Connection*? https://github.com/openstack/swift/blob/2.16.0/swift/proxy/controllers/base.py#L16-L25 has me feeling a little paranoid now... but then we\u0027ve got *this* comment, too!? https://github.com/openstack/swift/blob/2.16.0/swift/proxy/controllers/base.py#L711-L720\n\nI don\u0027t know *what* to feel... and why doesn\u0027t this take care of us? https://github.com/openstack/swift/blob/2.16.0/swift/proxy/controllers/base.py#L1105-L1108 Or is it that we *need* that, but we weren\u0027t getting there because we never closed the outer generator? And... somewhere... there\u0027s a ref still held so it never gets GCed...\n\nAnd looking at https://github.com/openstack/swift/blob/2.16.0/swift/proxy/controllers/base.py#L1245-L1257 I\u0027m worrying that we never called\n\n close_swift_conn(src)\n\nfor all the sources we filtered out at the top...","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"53549ef23f3f572043c8067b5d6e24eefdc9218f","unresolved":false,"context_lines":[{"line_number":7191,"context_line":"                return retval"},{"line_number":7192,"context_line":""},{"line_number":7193,"context_line":"            def close(self, *args, **kwargs):"},{"line_number":7194,"context_line":"                rv \u003d BufferedHTTPResponse.close(self, *args, **kwargs)"},{"line_number":7195,"context_line":"                unclosed_http_responses.pop(id(self), None)"},{"line_number":7196,"context_line":"                return rv"},{"line_number":7197,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_4b0f1b2e","line":7194,"in_reply_to":"9f91af0f_6056be09","updated":"2017-12-29 22:07:04.000000000","message":"Oh, better yet. Looks like that comment about \"Connection: close\" being present is wrong. Commit 6c320b2 removed that header from EC GET responses, but that was the only place it was present.\n\nI think it\u0027s only the close() of self._real_socket in BufferedHTTPResponse that does anything, or maybe the Response object is still becoming responsible for the connection in some other confusing way.","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"c723da55eaec1c39edbb3647bf9f7ee41820af1f","unresolved":false,"context_lines":[{"line_number":7191,"context_line":"                return retval"},{"line_number":7192,"context_line":""},{"line_number":7193,"context_line":"            def close(self, *args, **kwargs):"},{"line_number":7194,"context_line":"                rv \u003d BufferedHTTPResponse.close(self, *args, **kwargs)"},{"line_number":7195,"context_line":"                unclosed_http_responses.pop(id(self), None)"},{"line_number":7196,"context_line":"                return rv"},{"line_number":7197,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f91af0f_6056be09","line":7194,"in_reply_to":"9f91af0f_d7e3d7a3","updated":"2017-12-29 18:22:23.000000000","message":"Yeah, it was the ref held somewhere. Probably in ECAppIter, where it takes a generator that closes over self, then assigns it to self.stashed_iter. \n\nMight be worth investigating the lack of closure for all the filtered-out sources. My functional testing just had the one object with the right fragment archives on disk, so nothing would get filtered.","commit_id":"469500de3fc456d70e2323ec980e1a0ff4ad824f"}]}