)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0ce9306344743bd5756fa0d515f58e332923102d","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Instead of having to spin in the wait method, just use"},{"line_number":10,"context_line":"a condition and block until stopping has actually happened,"},{"line_number":11,"context_line":"when stop happens, it will use the notify_all method to let"},{"line_number":12,"context_line":"any blockers release."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #1505730"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"5a2ca52d_f3f199ce","line":12,"updated":"2015-10-14 09:57:32.000000000","message":"This is a *really* confusing commit message for the uninitiated, as nothing spins anywhere.","commit_id":"c68266b36b5d8e94fef2cca23ea459a01e05a0d2"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0ce9306344743bd5756fa0d515f58e332923102d","unresolved":false,"context_lines":[{"line_number":11,"context_line":"when stop happens, it will use the notify_all method to let"},{"line_number":12,"context_line":"any blockers release."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #1505730"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I3cfbe1bf02d451e379b1dcc23dacb0139c03be76"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"5a2ca52d_d3301d59","line":14,"updated":"2015-10-14 09:57:32.000000000","message":"This bug is a classic deadlock. The problem is lock design. The bug happens because we call tombstone.wait() in the executor while holding the state lock in the MessageHandlingServer. However, this same lock must be taken in order to call _executor.stop(), which is what provides the event tombstone.wait() is waiting for.\n\nThe problem is the creeping scope of MessageHandlingServer._state_lock. This lock is intended to protect the internal state of MessageHandlingServer. It is not relevant to _executor, but it is being held while _executor is called.\n\nThis deadlock can be fixed much more simply by just releasing the existing _state_lock before calling any _executor methods. I will follow up with a much simpler alternative patch.","commit_id":"c68266b36b5d8e94fef2cca23ea459a01e05a0d2"}],"oslo_messaging/server.py":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"3855e9a9fda1eac14caeb82612199b2b5a7b6741","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            # We are the lucky calling thread to wait on the executor to"},{"line_number":197,"context_line":"            # actually finish."},{"line_number":198,"context_line":"            executor.wait()"},{"line_number":199,"context_line":"            # Close listener connection after processing all messages"},{"line_number":200,"context_line":"            executor.listener.cleanup()"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def reset(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a2ca52d_18e7a0e5","line":199,"updated":"2015-10-14 01:26:57.000000000","message":"perhaps this should be in a try:finally?","commit_id":"38e7f71dee28cda251976f450c51fd7c4ae23045"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"cfc930196fbbf017b4c60a393b025aa2e9420821","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            # We are the lucky calling thread to wait on the executor to"},{"line_number":197,"context_line":"            # actually finish."},{"line_number":198,"context_line":"            executor.wait()"},{"line_number":199,"context_line":"            # Close listener connection after processing all messages"},{"line_number":200,"context_line":"            executor.listener.cleanup()"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def reset(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a2ca52d_cc4646bf","line":199,"in_reply_to":"5a2ca52d_18e7a0e5","updated":"2015-10-14 04:20:03.000000000","message":"Likely a good idea. Let me do that.","commit_id":"38e7f71dee28cda251976f450c51fd7c4ae23045"},{"author":{"_account_id":18805,"name":"Alexey Lebedev","email":"alebedev@mirantis.com","username":"binarin"},"change_message_id":"be668f37fd011ef74504037fbe7b971f4c6fd613","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        if self._executor is not None:"},{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":"        with self._state_cond:"},{"line_number":137,"context_line":"            if self._executor is not None:"},{"line_number":138,"context_line":"                return"},{"line_number":139,"context_line":"            try:"},{"line_number":140,"context_line":"                listener \u003d self.dispatcher._listen(self.transport)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_3c6ce107","line":137,"updated":"2015-11-13 12:55:57.000000000","message":"This condition will never be triggered, because we have the  same exact condition right at the beginning of the function. So one of those \u0027if\u0027s should be removed.","commit_id":"c68266b36b5d8e94fef2cca23ea459a01e05a0d2"},{"author":{"_account_id":18805,"name":"Alexey Lebedev","email":"alebedev@mirantis.com","username":"binarin"},"change_message_id":"58343aa1142d39d632bf34437babda09b0962c9b","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        if self._executor is not None:"},{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":"        with self._state_cond:"},{"line_number":137,"context_line":"            if self._executor is not None:"},{"line_number":138,"context_line":"                return"},{"line_number":139,"context_line":"            try:"},{"line_number":140,"context_line":"                listener \u003d self.dispatcher._listen(self.transport)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_b95e9e10","line":137,"in_reply_to":"da85f559_3c6ce107","updated":"2015-11-13 13:53:43.000000000","message":"Or if this is an optimization, a comment should probably be added here, to avoid confusion.","commit_id":"c68266b36b5d8e94fef2cca23ea459a01e05a0d2"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0ce9306344743bd5756fa0d515f58e332923102d","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            self._executor \u003d self._executor_cls(self.conf, listener,"},{"line_number":145,"context_line":"                                                self.dispatcher)"},{"line_number":146,"context_line":"            self._executor.start()"},{"line_number":147,"context_line":"            self._state_cond.notify_all()"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    def stop(self):"},{"line_number":150,"context_line":"        \"\"\"Stop handling incoming messages."}],"source_content_type":"text/x-python","patch_set":9,"id":"5a2ca52d_1cfa2743","line":147,"updated":"2015-10-14 09:57:32.000000000","message":"This is a noop. Nothing is waiting on the conditions set by this block.","commit_id":"c68266b36b5d8e94fef2cca23ea459a01e05a0d2"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0ce9306344743bd5756fa0d515f58e332923102d","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                self._executor.stop()"},{"line_number":161,"context_line":"            self._state_cond.notify_all()"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    def wait(self):"},{"line_number":164,"context_line":"        \"\"\"Wait for message processing to complete."},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        After calling stop(), there may still be some existing messages"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a2ca52d_70d1ffcc","line":163,"updated":"2015-10-14 09:57:32.000000000","message":"Aside: this method name is confusing, as this method isn\u0027t guaranteed to wait. Only a single thread will wait. A second thread will continue, despite _executor.wait() not having completed.\n\nNot an issue with this patch specifically, though, as it doesn\u0027t change this behaviour.","commit_id":"c68266b36b5d8e94fef2cca23ea459a01e05a0d2"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"41a3aa72226d32c3d7733f19fb946694e12a683e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                self._executor.stop()"},{"line_number":161,"context_line":"            self._state_cond.notify_all()"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    def wait(self):"},{"line_number":164,"context_line":"        \"\"\"Wait for message processing to complete."},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        After calling stop(), there may still be some existing messages"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a2ca52d_400988dc","line":163,"in_reply_to":"5a2ca52d_70d1ffcc","updated":"2015-10-14 12:44:36.000000000","message":"As I mentioned on IRC, I misread what the code did before. This is actually a regression.","commit_id":"c68266b36b5d8e94fef2cca23ea459a01e05a0d2"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0ce9306344743bd5756fa0d515f58e332923102d","unresolved":false,"context_lines":[{"line_number":200,"context_line":"            finally:"},{"line_number":201,"context_line":"                # Close listener connection after processing all messages"},{"line_number":202,"context_line":"                executor.listener.cleanup()"},{"line_number":203,"context_line":"                executor \u003d None"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def reset(self):"},{"line_number":206,"context_line":"        \"\"\"Reset service."}],"source_content_type":"text/x-python","patch_set":9,"id":"5a2ca52d_bca1b3ab","line":203,"updated":"2015-10-14 09:57:32.000000000","message":"nit: this is a noop. Made me look to see if there was a missing self._executor \u003d None, which there isn\u0027t. It\u0027s slightly confusing.","commit_id":"c68266b36b5d8e94fef2cca23ea459a01e05a0d2"}]}
