)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"595d654012bf0d11dd576a64929df079671a56a1","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        def decorated_function(_self, context, instance_uuid, *args, **kwargs):"},{"line_number":102,"context_line":"            sem \u003d self.locks.get(instance_uuid)"},{"line_number":103,"context_line":"            if not sem:"},{"line_number":104,"context_line":"                sem \u003d self.locks[instance_uuid] \u003d semaphore.Semaphore()"},{"line_number":105,"context_line":"            if not sem.acquire(blocking\u003dFalse):"},{"line_number":106,"context_line":"                LOG.info(_(\"serialize_on_instance: decorating: |%s|\"),"},{"line_number":107,"context_line":"                         function)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAC3%2F%2F8QQ%3D","line":104,"updated":"2011-12-16 18:44:18.000000000","message":"The test-and-set of the `locks` dict isn\u0027t atomic, so a potential race here as well.\n\nLooks like `dict.set_default` isn\u0027t atomic (see http://bugs.python.org/issue13521) so probably just easiest to protect access to the dict w/ a mutex.","commit_id":"1c89fc4e795940a57178740888a625c289509094"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"937606555c98df5a86b502e72805398cbe1008d4","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        def decorated_function(_self, context, instance_uuid, *args, **kwargs):"},{"line_number":102,"context_line":"            sem \u003d self.locks.get(instance_uuid)"},{"line_number":103,"context_line":"            if not sem:"},{"line_number":104,"context_line":"                sem \u003d self.locks[instance_uuid] \u003d semaphore.Semaphore()"},{"line_number":105,"context_line":"            if not sem.acquire(blocking\u003dFalse):"},{"line_number":106,"context_line":"                LOG.info(_(\"serialize_on_instance: decorating: |%s|\"),"},{"line_number":107,"context_line":"                         function)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAC3%2F%2F8Os%3D","line":104,"in_reply_to":"AAAAC3%2F%2F8QQ%3D","updated":"2011-12-16 20:34:59.000000000","message":"Really great point, thanks! WIP","commit_id":"1c89fc4e795940a57178740888a625c289509094"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"595d654012bf0d11dd576a64929df079671a56a1","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                sem.acquire()"},{"line_number":111,"context_line":"                LOG.info(_(\"serialize_on_instance: obtained: |%s|\"),"},{"line_number":112,"context_line":"                         instance_uuid)"},{"line_number":113,"context_line":"            LOG.info(_(\"serialize_on_instance: executing: |%s|\"), function)"},{"line_number":114,"context_line":"            try:"},{"line_number":115,"context_line":"                function(_self, context, instance_uuid, *args, **kwargs)"},{"line_number":116,"context_line":"            finally:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAC3%2F%2F8QI%3D","line":113,"updated":"2011-12-16 18:44:18.000000000","message":"I think technically this should go in the `try`, since it\u0027s within the critical section.","commit_id":"1c89fc4e795940a57178740888a625c289509094"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"abb8645faba1bbba2afc8512c2eaeeea916584a5","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                sem.acquire()"},{"line_number":111,"context_line":"                LOG.info(_(\"serialize_on_instance: obtained: |%s|\"),"},{"line_number":112,"context_line":"                         instance_uuid)"},{"line_number":113,"context_line":"            LOG.info(_(\"serialize_on_instance: executing: |%s|\"), function)"},{"line_number":114,"context_line":"            try:"},{"line_number":115,"context_line":"                function(_self, context, instance_uuid, *args, **kwargs)"},{"line_number":116,"context_line":"            finally:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAC3%2F%2F8N0%3D","line":113,"in_reply_to":"AAAAC3%2F%2F8Oo%3D","updated":"2011-12-16 20:55:28.000000000","message":"Anything after `acquire()` must be guaranteed to release the lock or we could deadlock.\n\nSince LOG could, in theory, write over a network socket, there\u0027s always the risk of a TimeoutError or the like. Since, as written, that would not release the lock, we could end up in a bad-state.\n\nTo mitigate that, all we need to do is make sure any statements after a lock as been acquired are within the try/finally (or `with` block):\n\nsem.acquire()\ntry:\n    LOG.info(\"blah\")\n    function(*args, **kwargs)\nfinally:\n    sem.release()","commit_id":"1c89fc4e795940a57178740888a625c289509094"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"937606555c98df5a86b502e72805398cbe1008d4","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                sem.acquire()"},{"line_number":111,"context_line":"                LOG.info(_(\"serialize_on_instance: obtained: |%s|\"),"},{"line_number":112,"context_line":"                         instance_uuid)"},{"line_number":113,"context_line":"            LOG.info(_(\"serialize_on_instance: executing: |%s|\"), function)"},{"line_number":114,"context_line":"            try:"},{"line_number":115,"context_line":"                function(_self, context, instance_uuid, *args, **kwargs)"},{"line_number":116,"context_line":"            finally:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAC3%2F%2F8Oo%3D","line":113,"in_reply_to":"AAAAC3%2F%2F8QI%3D","updated":"2011-12-16 20:34:59.000000000","message":"I don\u0027t really understand this, can you elaborate on your reasoning?","commit_id":"1c89fc4e795940a57178740888a625c289509094"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"79c4cd031c4d00daebfd57a602909573713aef5f","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                     function.__name__, instance_uuid)"},{"line_number":104,"context_line":"            lock \u003d self._get_instance_lock(instance_uuid)"},{"line_number":105,"context_line":"            if not lock.acquire(blocking\u003dFalse):"},{"line_number":106,"context_line":"                lock.acquire()"},{"line_number":107,"context_line":"            try:"},{"line_number":108,"context_line":"                LOG.info(_(\"serialize_on_instance: executing: |%s| |%s|\"),"},{"line_number":109,"context_line":"                         function.__name__, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F8Lc%3D","line":106,"updated":"2011-12-16 22:15:08.000000000","message":"Now that we\u0027re not logging if we don\u0027t acquire the lock, I think this can go away and just become:\n\nlock.acquire().","commit_id":"0a6827010d87d817d5b06f6084ee71e276db13e6"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"79c4cd031c4d00daebfd57a602909573713aef5f","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            try:"},{"line_number":108,"context_line":"                LOG.info(_(\"serialize_on_instance: executing: |%s| |%s|\"),"},{"line_number":109,"context_line":"                         function.__name__, instance_uuid)"},{"line_number":110,"context_line":"                function(_self, context, instance_uuid, *args, **kwargs)"},{"line_number":111,"context_line":"            finally:"},{"line_number":112,"context_line":"                lock.release()"},{"line_number":113,"context_line":"                LOG.info(_(\"serialize_on_instance: released lock: |%s| |%s|\"),"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F8LY%3D","line":110,"updated":"2011-12-16 22:15:08.000000000","message":"I think you\u0027re missing a `return` here so that the decorated-function returns whatever the inner function returns.","commit_id":"0a6827010d87d817d5b06f6084ee71e276db13e6"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"79c4cd031c4d00daebfd57a602909573713aef5f","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        return decorated_function"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def _get_instance_lock(self, instance_uuid):"},{"line_number":118,"context_line":"        # NOTE: This function is only atomic because we are using lightweight"},{"line_number":119,"context_line":"        # threads. If we switch to using native threads, we will need to make"},{"line_number":120,"context_line":"        # this function explicitly atomic"},{"line_number":121,"context_line":"        if not instance_uuid in self.locks:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F8Lk%3D","line":118,"updated":"2011-12-16 22:15:08.000000000","message":"Per HACKING, this should be NOTE(markwash) or the like.","commit_id":"0a6827010d87d817d5b06f6084ee71e276db13e6"}]}