)]}'
{"oslo_service/loopingcall.py":[{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"788ec8f8aff9487bc2be1526799795dd5933ee1d","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    return func"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"class _Event(object):"},{"line_number":80,"context_line":"    def __init__(self):"},{"line_number":81,"context_line":"        self._abort \u003d eventletutils.Event()"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_860973dd","line":79,"updated":"2018-10-19 21:11:58.000000000","message":"This class probably doesn\u0027t need to exist any more; it was added by If715fbe21ac085e4f5c83cef0729dbca8dcb19ca to abstract between the two behaviours. The original patch I5835f9595826df5349e4cc8b1da8529bb960ee04 has this logic directly inside the LoopingCallBase; all we really needed to do was replace threading.Event() with eventletutils.Event() in that patch.","commit_id":"f88ed34b8b57fb739648173372e77a7e702bbec9"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"125c742bfb0db58bb0ca92a01bbbb04bdd24f100","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        self.f \u003d f"},{"line_number":89,"context_line":"        self._thread \u003d None"},{"line_number":90,"context_line":"        self.done \u003d None"},{"line_number":91,"context_line":"        self._event \u003d eventletutils.Event()"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    @property"},{"line_number":94,"context_line":"    def _running(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_83e14c2e","line":91,"updated":"2018-10-22 13:44:37.000000000","message":"nitpick: in I5835f9595826df5349e4cc8b1da8529bb960ee04 this was called self._abort and that was slightly clearer in explaining what\u0027s happening here.","commit_id":"b20419e49db285c511d00915bf485a0d965b2eec"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"8e7924c5a0efaf5fbfdc45d3219dff9befe0b91d","unresolved":false,"context_lines":[{"line_number":79,"context_line":"class _Event(object):"},{"line_number":80,"context_line":"    \"\"\"A class that provides consistent eventlet/threading Event API."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    It\u0027s a copy from the oslo_utils repository, as we need the private version"},{"line_number":83,"context_line":"    because we don\u0027t want to use the threading.Event version."},{"line_number":84,"context_line":"    \"\"\""},{"line_number":85,"context_line":"    def __init__(self, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_60309bea","line":82,"updated":"2018-10-22 19:49:07.000000000","message":"I wonder if we should make it public in oslo.utils. It\u0027s just a wrapper that implements a stdlib interface, so it\u0027s unlikely to change, and if it did it would only be because the stdlib changed. It\u0027s not really private anyway since instances of it are returned to users, so we can\u0027t monkey with the interface.","commit_id":"c2463470f5e7edad1ffd369a472267d559590495"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"309ea6415da9ee4d70021bdb9601d2e13ca2aa72","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    def is_set(self):"},{"line_number":93,"context_line":"        return self._set"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def set(self):"},{"line_number":96,"context_line":"        self._set \u003d True"},{"line_number":97,"context_line":"        self._event.send(True)"},{"line_number":98,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_c8d8cdee","line":95,"updated":"2018-11-02 17:12:26.000000000","message":"It seems like this code has been copied from oslo_utils.eventletutils, but without this fix:\nhttps://github.com/openstack/oslo.utils/commit/82678918aab77f946da3a13f40fe8731c54e8bd4","commit_id":"c2463470f5e7edad1ffd369a472267d559590495"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"5f7a8b3628d9d02208269efa3539b586840d87da","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    def is_set(self):"},{"line_number":93,"context_line":"        return self._set"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def set(self):"},{"line_number":96,"context_line":"        self._set \u003d True"},{"line_number":97,"context_line":"        self._event.send(True)"},{"line_number":98,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_8324f6b2","line":95,"in_reply_to":"3f79a3b5_c8d8cdee","updated":"2018-11-02 17:20:36.000000000","message":"Oh, it likely explains this bug:\nhttps://bugs.launchpad.net/neutron/+bug/1800879\n\nThis bug has been worked around by modifying LoopingCallBase.stop():\nhttps://review.openstack.org/#/c/614642/","commit_id":"c2463470f5e7edad1ffd369a472267d559590495"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"309ea6415da9ee4d70021bdb9601d2e13ca2aa72","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def wait(self, timeout\u003dNone):"},{"line_number":100,"context_line":"        with eventlettimeout.Timeout(timeout, False):"},{"line_number":101,"context_line":"            self._event.wait()"},{"line_number":102,"context_line":"        return self.is_set()"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_bd159197","line":101,"updated":"2018-11-02 17:12:26.000000000","message":"I have been asked to review the backport to Rocky:\nhttps://review.openstack.org/#/c/614489/\n\nIs there a race condition on this line in a greenthread A is blocked on the wait, but another greenthread B calls clear() and then set()? B calls self._event.send(), but A is waiting on a different eventlet Event which is no longer used by the oslo.service Event...\n\neventlet.event.Event implements a reset() method but it\u0027s documented as broken and suggest to create a new Event object:\n\n        # this is kind of a misfeature and doesn\u0027t work perfectly well,\n        # it\u0027s better to create a new event rather than reset an old one\n# removing documentation so that we don\u0027t get new use cases for it\n\nhttps://github.com/eventlet/eventlet/blob/master/eventlet/event.py#L53\n\nIf there is a race condition, maybe we need something like:\n\ndef clear(self):\n    old_event \u003d self._event\n    self._set \u003d False\n    self._event \u003d event.Event()\n    # Unblock any greenthreads blocked in wait()\n    old_event.send(True)\n\ndef wait():\n    event \u003d self._event\n    while True:\n        event.wait()\n        if event is not self._event:\n            # clear has been called, retry\n        break\n    ...\n\nAnd now something else, this class seems to be copied from oslo.utils:\nhttps://github.com/openstack/oslo.utils/blob/c568717706b18eab1c679c6792b8016f5fbb9a35/oslo_utils/eventletutils.py#L143-L171\n\nThe code in oslo.utils seems to have the same race condition. Why this code has been copied, instead of just reusing oslo_utils.eventletutils._Event? Is it because the symbol is private? If yes, I suggest to rename _Event to GreenEvent. Ben asked asked if the oslo_utils.eventletutils class should be exposed.\n\nCopying the code doesn\u0027t seem like a good idea, since we miss fixes like:\nhttps://github.com/openstack/oslo.utils/commit/82678918aab77f946da3a13f40fe8731c54e8bd4","commit_id":"c2463470f5e7edad1ffd369a472267d559590495"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"9228ca4432186fb0636f377421baf0bbcd245273","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def wait(self, timeout\u003dNone):"},{"line_number":100,"context_line":"        with eventlettimeout.Timeout(timeout, False):"},{"line_number":101,"context_line":"            self._event.wait()"},{"line_number":102,"context_line":"        return self.is_set()"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_84184b1f","line":101,"in_reply_to":"3f79a3b5_bd159197","updated":"2018-11-21 19:07:07.000000000","message":"\u003e Is there a race condition on this line\n\nLooks like yes, which is a problem for eventletutils, but the way it\u0027s being used here will never encounter that race. (clear() is only called once, before the looping call is started. If you try to start it again while it\u0027s still running, a RuntimeError will be raised on line 149 before clear() can be called.)\n\n\u003e Ben asked asked if the oslo_utils.eventletutils class should be exposed.\n\nIf8e7a41f9fe5573a780f9faabdbaf1326631d37a\nIf0dfac2505d097c117ef94c99399b1614f1e1f8f","commit_id":"c2463470f5e7edad1ffd369a472267d559590495"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"8e7924c5a0efaf5fbfdc45d3219dff9befe0b91d","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        \"\"\""},{"line_number":148,"context_line":"        if self._thread is not None:"},{"line_number":149,"context_line":"            raise RuntimeError(self._RUN_ONLY_ONE_MESSAGE)"},{"line_number":150,"context_line":"        self.done \u003d event.Event()"},{"line_number":151,"context_line":"        self._abort.clear()"},{"line_number":152,"context_line":"        self._thread \u003d greenthread.spawn("},{"line_number":153,"context_line":"            self._run_loop, idle_for,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_e0090bdc","line":150,"updated":"2018-10-22 19:49:07.000000000","message":"Note that we were already unconditionally using an eventlet Event.","commit_id":"c2463470f5e7edad1ffd369a472267d559590495"}]}
