)]}'
{"swift/common/utils.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c40b836452b98118e9b940827151420c5dbf7c10","unresolved":false,"context_lines":[{"line_number":4312,"context_line":"_eventfd \u003d None"},{"line_number":4313,"context_line":""},{"line_number":4314,"context_line":""},{"line_number":4315,"context_line":"class EventFdMutex(object):"},{"line_number":4316,"context_line":"    \"\"\""},{"line_number":4317,"context_line":"    Mutex using an eventfd. Works across both greenlets and real threads,"},{"line_number":4318,"context_line":"    even at the same time."}],"source_content_type":"text/x-python","patch_set":1,"id":"9f436f4f_76cdb864","line":4315,"updated":"2017-08-14 23:31:30.000000000","message":"How about something more like\n\n class GreenPthreadRLock(object):\n     def __init__(self):\n         self.posixlock \u003d stdlib_threading.RLock()\n         self.dictlock \u003d stdlib_threading.Lock()\n         self.greenlocks \u003d collections.defaultdict(threading.RLock)\n\n     @property\n     def greenlock(self):\n         self.dictlock.acquire()\n         try:\n             return self.greenlocks[stdlib_threading.current_thread()]\n         finally:\n             self.dictlock.release()\n\n     def acquire(self, blocking\u003dTrue):\n         while True:\n             if not self.greenlock.acquire(blocking):\n                 return False\n             if self.posixlock.acquire(False):\n                 return True\n             self.greenlock.release()\n             if not blocking:\n                 return False\n             sleep(0)\n\n     def release(self):\n         self.posixlock.release()\n         self.greenlock.release()\n\n...and we get rid of all the close() badness? There\u0027s still the chance for a tight loop if we have, say, two pthreads, each with one greenthread, and one of the greenthreads holds the lock for a while, but I think in this particular context, no one should be holding the lock very long? Alternatively, maybe we sleep for 1ms or something, which still lets the common case (no one else is holding the lock) be pretty fast.","commit_id":"2cb23503f42aa01eaa4d873200522ae031b5a378"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"511a4cbe6067cb231446f08bb13b75862b1184ee","unresolved":false,"context_lines":[{"line_number":4312,"context_line":"_eventfd \u003d None"},{"line_number":4313,"context_line":""},{"line_number":4314,"context_line":""},{"line_number":4315,"context_line":"class EventFdMutex(object):"},{"line_number":4316,"context_line":"    \"\"\""},{"line_number":4317,"context_line":"    Mutex using an eventfd. Works across both greenlets and real threads,"},{"line_number":4318,"context_line":"    even at the same time."}],"source_content_type":"text/x-python","patch_set":1,"id":"9f436f4f_4172f07c","line":4315,"in_reply_to":"9f436f4f_76cdb864","updated":"2017-08-15 00:31:25.000000000","message":"I\u0027m not a big fan of the busy-wait in acquire() there. Other than that, I\u0027m in favor of re-using existing locking primitives instead of inventing our own, if we can do it without any busy-wait loops.","commit_id":"2cb23503f42aa01eaa4d873200522ae031b5a378"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1ff897edcf2d5a0c0dbfe85189c250cf4a01ef07","unresolved":false,"context_lines":[{"line_number":4405,"context_line":"                # Tell eventlet to suspend the current greenthread until"},{"line_number":4406,"context_line":"                # this file descriptor becomes readable. This will happen"},{"line_number":4407,"context_line":"                # when someone else writes a non-zero value to it."},{"line_number":4408,"context_line":"                trampoline(self.fd, read\u003dTrue)"},{"line_number":4409,"context_line":""},{"line_number":4410,"context_line":"    def release(self):"},{"line_number":4411,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f436f4f_7334af93","line":4408,"updated":"2017-08-14 18:49:21.000000000","message":"$#$% just got *real*","commit_id":"2cb23503f42aa01eaa4d873200522ae031b5a378"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3d021fc4a15dd1bb6d094f354a18a9389026381","unresolved":false,"context_lines":[{"line_number":55,"context_line":"import eventlet.semaphore"},{"line_number":56,"context_line":"from eventlet import GreenPool, sleep, Timeout, tpool"},{"line_number":57,"context_line":"from eventlet.green import socket, threading"},{"line_number":58,"context_line":"from eventlet.hubs import trampoline"},{"line_number":59,"context_line":"import eventlet.queue"},{"line_number":60,"context_line":"import netifaces"},{"line_number":61,"context_line":"import codecs"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f436f4f_9c3efb88","line":58,"updated":"2017-08-15 01:43:48.000000000","message":"I\u0027m a little nervous about relying on something that documents itself [1] as\n\n\u003e an internal API, and it might change unexpectedly without being deprecated first\n\nbut I suppose it\u0027s probably unlikely to break?\n\n[1] http://eventlet.net/doc/hubs.html?highlight\u003dtrampoline#eventlet.hubs.trampoline","commit_id":"a3af663e0be2c4d4c5958b7bd7b200293a4d2218"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3d021fc4a15dd1bb6d094f354a18a9389026381","unresolved":false,"context_lines":[{"line_number":4394,"context_line":"        too."},{"line_number":4395,"context_line":"        \"\"\""},{"line_number":4396,"context_line":"        current_greenthread_id \u003d id(eventlet.greenthread.getcurrent())"},{"line_number":4397,"context_line":"        if self.owner \u003d\u003d current_greenthread_id:"},{"line_number":4398,"context_line":"            self.recursion_depth +\u003d 1"},{"line_number":4399,"context_line":"            return True"},{"line_number":4400,"context_line":""},{"line_number":4401,"context_line":"        while True:"},{"line_number":4402,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f436f4f_1cee8bec","line":4399,"range":{"start_line":4397,"start_character":0,"end_line":4399,"end_character":23},"updated":"2017-08-15 01:43:48.000000000","message":"...and this sort of stuff is thread safe because we can only cede priority within this pthread at\n\n- os.read\n- trampoline, and\n- os.write\n\n...yeah? And the os.read / os.write protects us *between* pthreads.","commit_id":"a3af663e0be2c4d4c5958b7bd7b200293a4d2218"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"06c0d025a5856a3036ed608813455e7b7a277b57","unresolved":false,"context_lines":[{"line_number":4396,"context_line":"        current_greenthread_id \u003d id(eventlet.greenthread.getcurrent())"},{"line_number":4397,"context_line":"        if self.owner \u003d\u003d current_greenthread_id:"},{"line_number":4398,"context_line":"            self.recursion_depth +\u003d 1"},{"line_number":4399,"context_line":"            return True"},{"line_number":4400,"context_line":""},{"line_number":4401,"context_line":"        while True:"},{"line_number":4402,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f436f4f_7970f208","line":4399,"in_reply_to":"9f436f4f_1cee8bec","updated":"2017-08-15 17:19:21.000000000","message":"Mostly. If we\u0027re the owner, then we hold this lock, and anyone else coming in here at the same time has to be another greenthread in our pthread. However, they\u0027ll see self.owner !\u003d current_greenthread_id, so they won\u0027t come in here.\n\nAlso, os.read and os.write aren\u0027t monkeypatched by eventlet, and there\u0027s no other IO in here, so the only context-switch site is the trampoline() call.","commit_id":"a3af663e0be2c4d4c5958b7bd7b200293a4d2218"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3d021fc4a15dd1bb6d094f354a18a9389026381","unresolved":false,"context_lines":[{"line_number":4420,"context_line":"                # when someone else writes a non-zero value to it."},{"line_number":4421,"context_line":"                trampoline(self.fd, read\u003dTrue)"},{"line_number":4422,"context_line":""},{"line_number":4423,"context_line":"    def release(self):"},{"line_number":4424,"context_line":"        \"\"\""},{"line_number":4425,"context_line":"        Release the mutex."},{"line_number":4426,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f436f4f_3c7caf1d","line":4423,"updated":"2017-08-15 01:43:48.000000000","message":"Since we have ownership information now, should we include some checks that a lock is unlocked by its owner? (Possibly with a `force\u003dFalse` kwarg you could override to allow it.)","commit_id":"a3af663e0be2c4d4c5958b7bd7b200293a4d2218"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"06c0d025a5856a3036ed608813455e7b7a277b57","unresolved":false,"context_lines":[{"line_number":4420,"context_line":"                # when someone else writes a non-zero value to it."},{"line_number":4421,"context_line":"                trampoline(self.fd, read\u003dTrue)"},{"line_number":4422,"context_line":""},{"line_number":4423,"context_line":"    def release(self):"},{"line_number":4424,"context_line":"        \"\"\""},{"line_number":4425,"context_line":"        Release the mutex."},{"line_number":4426,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f436f4f_39f6ea64","line":4423,"in_reply_to":"9f436f4f_3c7caf1d","updated":"2017-08-15 17:19:21.000000000","message":"I don\u0027t think it would be too helpful. This mutex is only getting used down in the internals of logging, so even if we did find a bug where A holds the mutex but B unlocks it, what would we do about it? Send a patch upstream and wait until we\u0027re ported to Python 3.7?","commit_id":"a3af663e0be2c4d4c5958b7bd7b200293a4d2218"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3d021fc4a15dd1bb6d094f354a18a9389026381","unresolved":false,"context_lines":[{"line_number":4435,"context_line":"        \"\"\""},{"line_number":4436,"context_line":"        Close the mutex. This releases its file descriptor."},{"line_number":4437,"context_line":""},{"line_number":4438,"context_line":"        You can\u0027t use a mutex after it\u0027s been closed."},{"line_number":4439,"context_line":"        \"\"\""},{"line_number":4440,"context_line":"        if self.fd is not None:"},{"line_number":4441,"context_line":"            os.close(self.fd)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f436f4f_5ca413a6","line":4438,"range":{"start_line":4438,"start_character":8,"end_line":4438,"end_character":53},"updated":"2017-08-15 01:43:48.000000000","message":"What does the error look like if you do?","commit_id":"a3af663e0be2c4d4c5958b7bd7b200293a4d2218"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"06c0d025a5856a3036ed608813455e7b7a277b57","unresolved":false,"context_lines":[{"line_number":4435,"context_line":"        \"\"\""},{"line_number":4436,"context_line":"        Close the mutex. This releases its file descriptor."},{"line_number":4437,"context_line":""},{"line_number":4438,"context_line":"        You can\u0027t use a mutex after it\u0027s been closed."},{"line_number":4439,"context_line":"        \"\"\""},{"line_number":4440,"context_line":"        if self.fd is not None:"},{"line_number":4441,"context_line":"            os.close(self.fd)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f436f4f_9975d6fc","line":4438,"in_reply_to":"9f436f4f_5ca413a6","updated":"2017-08-15 17:19:21.000000000","message":"It\u0027s kind of ugly. os.read(None, 8) raises a TypeError.","commit_id":"a3af663e0be2c4d4c5958b7bd7b200293a4d2218"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3d021fc4a15dd1bb6d094f354a18a9389026381","unresolved":false,"context_lines":[{"line_number":4445,"context_line":""},{"line_number":4446,"context_line":"    def __del__(self):"},{"line_number":4447,"context_line":"        # We need this so we don\u0027t leak file descriptors. Otherwise, if you"},{"line_number":4448,"context_line":"        # call get_logger() and don\u0027t explicitly dispose of it by calling"},{"line_number":4449,"context_line":"        # logger.logger.handlers[0].lock.close(), the eventfd file"},{"line_number":4450,"context_line":"        # descriptor is leaked."},{"line_number":4451,"context_line":"        #"},{"line_number":4452,"context_line":"        # This only really comes up in tests. Swift processes tend to call"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f436f4f_9c25db5e","line":4449,"range":{"start_line":4448,"start_character":28,"end_line":4449,"end_character":48},"updated":"2017-08-15 01:43:48.000000000","message":"...which isn\u0027t really part of the normal Lock API, right? So we can\u0027t be *too* surprised if consumers don\u0027t do it...","commit_id":"a3af663e0be2c4d4c5958b7bd7b200293a4d2218"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"06c0d025a5856a3036ed608813455e7b7a277b57","unresolved":false,"context_lines":[{"line_number":4446,"context_line":"    def __del__(self):"},{"line_number":4447,"context_line":"        # We need this so we don\u0027t leak file descriptors. Otherwise, if you"},{"line_number":4448,"context_line":"        # call get_logger() and don\u0027t explicitly dispose of it by calling"},{"line_number":4449,"context_line":"        # logger.logger.handlers[0].lock.close(), the eventfd file"},{"line_number":4450,"context_line":"        # descriptor is leaked."},{"line_number":4451,"context_line":"        #"},{"line_number":4452,"context_line":"        # This only really comes up in tests. Swift processes tend to call"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f436f4f_f95d0279","line":4449,"in_reply_to":"9f436f4f_9c25db5e","updated":"2017-08-15 17:19:21.000000000","message":"Oh, certainly not. I can add a comment to that effect.","commit_id":"a3af663e0be2c4d4c5958b7bd7b200293a4d2218"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e67b4b4fe9db1a11f7db2bfcfa251d9a876c742d","unresolved":false,"context_lines":[{"line_number":4429,"context_line":"            return"},{"line_number":4430,"context_line":""},{"line_number":4431,"context_line":"        os.write(self.fd, self.SOME_NONZERO_VALUE)"},{"line_number":4432,"context_line":"        self.owner \u003d None"},{"line_number":4433,"context_line":""},{"line_number":4434,"context_line":"    def close(self):"},{"line_number":4435,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f436f4f_a5498d60","line":4432,"updated":"2017-08-15 19:52:57.000000000","message":"I think we want this *before* os.write -- otherwise the lock may not be reentrant.\n\n- pthread 1 grabs the lock\n- pthread 2 wants the lock\n- pthread 1 calls release, gets as far as os.write, then gets preempted\n- pthread 2 wakes up, gets the lock, assigned itself as owner\n- some time later, pthread 1 wakes up, clears owner, and continues\n- pthread 2 deadlocks while trying to grab the lock a second time\n\nI think this should demonstrate it:\n\n diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py\n index e14ca562..05647498 100644\n --- a/test/unit/common/test_utils.py\n +++ b/test/unit/common/test_utils.py\n @@ -6350,6 +6350,44 @@ class TestEventFdMutex(unittest.TestCase):\n  \n          self.assertEqual(\u0027\u0027.join(sequence), \"\u003c\u003e\" * 40)\n  \n +    def test_blocking_preserves_ownership(self):\n +        pthread1_event \u003d eventlet.patcher.original(\u0027threading\u0027).Event()\n +        pthread2_event \u003d eventlet.patcher.original(\u0027threading\u0027).Event()\n +        owner \u003d []\n +\n +        def pthread1():\n +            self.mutex.acquire()\n +            orig_os_write \u003d utils.os.write\n +\n +            def patched_os_write(*a, **kw):\n +                try:\n +                    return orig_os_write(*a, **kw)\n +                finally:\n +                    pthread1_event.wait()\n +\n +            with mock.patch.object(utils.os, \u0027write\u0027, patched_os_write):\n +                self.mutex.release()\n +            pthread2_event.set()\n +\n +        def pthread2():\n +            self.mutex.acquire()\n +            pthread1_event.set()\n +            pthread2_event.wait()\n +            owner.append(self.mutex.owner)\n +            self.mutex.release()\n +\n +        real_thread1 \u003d eventlet.patcher.original(\u0027threading\u0027).Thread(\n +            target\u003dpthread1)\n +        real_thread1.start()\n +\n +        real_thread2 \u003d eventlet.patcher.original(\u0027threading\u0027).Thread(\n +            target\u003dpthread2)\n +        real_thread2.start()\n +\n +        real_thread1.join()\n +        real_thread2.join()\n +        self.assertIsNotNone(owner[0])\n +\n      @classmethod\n      def tearDownClass(cls):\n          # EventFdMutex turns this off when you instantiate one","commit_id":"875e232307d483d9a76828765e1eb50573be2eb2"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"b84ee79e7424a8fd102bdf998917855695ec94de","unresolved":false,"context_lines":[{"line_number":4429,"context_line":"            return"},{"line_number":4430,"context_line":""},{"line_number":4431,"context_line":"        os.write(self.fd, self.SOME_NONZERO_VALUE)"},{"line_number":4432,"context_line":"        self.owner \u003d None"},{"line_number":4433,"context_line":""},{"line_number":4434,"context_line":"    def close(self):"},{"line_number":4435,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f436f4f_e9940f39","line":4432,"in_reply_to":"9f436f4f_a5498d60","updated":"2017-08-15 20:41:47.000000000","message":"Done","commit_id":"875e232307d483d9a76828765e1eb50573be2eb2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9e2bf49016e60ede31f298e62d59c52414e19221","unresolved":false,"context_lines":[{"line_number":92,"context_line":"# setup notice level logging"},{"line_number":93,"context_line":"NOTICE \u003d 25"},{"line_number":94,"context_line":"logging.addLevelName(NOTICE, \u0027NOTICE\u0027)"},{"line_number":95,"context_line":"SysLogHandler.priority_map[\u0027NOTICE\u0027] \u003d \u0027notice\u0027"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"# These are lazily pulled from libc elsewhere"},{"line_number":98,"context_line":"_sys_fallocate \u003d None"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_0966332b","line":95,"updated":"2017-08-15 21:13:39.000000000","message":"Should we update ThreadSafeSysLogHandler instead now?","commit_id":"31a9daac2e587df007e0c5b390dab5d6efdbc780"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cdabdf59658e3e2211b6b98dbe2c11509b7b5386","unresolved":false,"context_lines":[{"line_number":4323,"context_line":"        # later."},{"line_number":4324,"context_line":"        rflags \u003d fcntl.fcntl(self.rfd, fcntl.F_GETFL)"},{"line_number":4325,"context_line":"        fcntl.fcntl(self.rfd, fcntl.F_SETFL, rflags | os.O_NONBLOCK)"},{"line_number":4326,"context_line":"        os.write(self.wfd, \u0027-\u0027)  # start unlocked"},{"line_number":4327,"context_line":""},{"line_number":4328,"context_line":"        self.owner \u003d None"},{"line_number":4329,"context_line":"        self.recursion_depth \u003d 0"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_cf970b3a","line":4326,"range":{"start_line":4326,"start_character":27,"end_line":4326,"end_character":30},"updated":"2017-08-15 22:31:35.000000000","message":"Mind being explicit about the type here?\n\n b\u0027-\u0027","commit_id":"8557dbcf2af250ce0ab1c5a429ca28c53b553dd4"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"98ef3744efa2129973cbfd3b27294a23eca30193","unresolved":false,"context_lines":[{"line_number":4323,"context_line":"        # later."},{"line_number":4324,"context_line":"        rflags \u003d fcntl.fcntl(self.rfd, fcntl.F_GETFL)"},{"line_number":4325,"context_line":"        fcntl.fcntl(self.rfd, fcntl.F_SETFL, rflags | os.O_NONBLOCK)"},{"line_number":4326,"context_line":"        os.write(self.wfd, \u0027-\u0027)  # start unlocked"},{"line_number":4327,"context_line":""},{"line_number":4328,"context_line":"        self.owner \u003d None"},{"line_number":4329,"context_line":"        self.recursion_depth \u003d 0"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_6f5dbf45","line":4326,"in_reply_to":"9f436f4f_cf970b3a","updated":"2017-08-15 22:38:26.000000000","message":"Done","commit_id":"8557dbcf2af250ce0ab1c5a429ca28c53b553dd4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cdabdf59658e3e2211b6b98dbe2c11509b7b5386","unresolved":false,"context_lines":[{"line_number":4390,"context_line":"            return"},{"line_number":4391,"context_line":""},{"line_number":4392,"context_line":"        self.owner \u003d None"},{"line_number":4393,"context_line":"        os.write(self.wfd, \u0027X\u0027)"},{"line_number":4394,"context_line":""},{"line_number":4395,"context_line":"    def close(self):"},{"line_number":4396,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_8fa583a4","line":4393,"updated":"2017-08-15 22:31:35.000000000","message":"...and here","commit_id":"8557dbcf2af250ce0ab1c5a429ca28c53b553dd4"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"98ef3744efa2129973cbfd3b27294a23eca30193","unresolved":false,"context_lines":[{"line_number":4390,"context_line":"            return"},{"line_number":4391,"context_line":""},{"line_number":4392,"context_line":"        self.owner \u003d None"},{"line_number":4393,"context_line":"        os.write(self.wfd, \u0027X\u0027)"},{"line_number":4394,"context_line":""},{"line_number":4395,"context_line":"    def close(self):"},{"line_number":4396,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_8f622309","line":4393,"in_reply_to":"9f436f4f_8fa583a4","updated":"2017-08-15 22:38:26.000000000","message":"Done","commit_id":"8557dbcf2af250ce0ab1c5a429ca28c53b553dd4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8c67f5bda4e1d4e3d9e32524bf8c6b59d20f8ef4","unresolved":false,"context_lines":[{"line_number":4384,"context_line":"    def release(self):"},{"line_number":4385,"context_line":"        \"\"\""},{"line_number":4386,"context_line":"        Release the mutex."},{"line_number":4387,"context_line":"        \"\"\""},{"line_number":4388,"context_line":"        if self.recursion_depth \u003e 0:"},{"line_number":4389,"context_line":"            self.recursion_depth -\u003d 1"},{"line_number":4390,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_42252008","line":4387,"updated":"2017-08-16 15:46:53.000000000","message":"should we be testing that the owner is the current thread?\notherwise, what is the owner attribute used for?\n\nright now this isn\u0027t a big deal because the only caller is logging and we can assume it is acquiring/releasing correctly, but in future someone might use this class for other purposes and detecting unexpected releases might be useful.","commit_id":"3187ce04d6ba78380e9046673dd47f9d6aebdad6"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"da743a81865ae7f0096b357df0808e67e668cdd3","unresolved":false,"context_lines":[{"line_number":4384,"context_line":"    def release(self):"},{"line_number":4385,"context_line":"        \"\"\""},{"line_number":4386,"context_line":"        Release the mutex."},{"line_number":4387,"context_line":"        \"\"\""},{"line_number":4388,"context_line":"        if self.recursion_depth \u003e 0:"},{"line_number":4389,"context_line":"            self.recursion_depth -\u003d 1"},{"line_number":4390,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_014eedbf","line":4387,"in_reply_to":"9f436f4f_42252008","updated":"2017-08-16 21:00:06.000000000","message":"I can add a check for that.\n\nI had originally thought that threading._RLock didn\u0027t have the ownership check on release(), and I didn\u0027t want to be stricter than the thing we\u0027re replacing. However, I now see that there is a check in there, so I\u0027m okay with adding it here too.","commit_id":"3187ce04d6ba78380e9046673dd47f9d6aebdad6"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d2aa9446fc3ab0dfcd663b23f08a1ec902580d01","unresolved":false,"context_lines":[{"line_number":4386,"context_line":"        Release the mutex."},{"line_number":4387,"context_line":"        \"\"\""},{"line_number":4388,"context_line":"        current_greenthread_id \u003d id(eventlet.greenthread.getcurrent())"},{"line_number":4389,"context_line":"        if self.owner !\u003d current_greenthread_id:"},{"line_number":4390,"context_line":"            raise RuntimeError(\"cannot release un-acquired lock\")"},{"line_number":4391,"context_line":""},{"line_number":4392,"context_line":"        if self.recursion_depth \u003e 0:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f436f4f_88655db7","line":4389,"updated":"2017-08-17 04:55:17.000000000","message":"Good, now we can\u0027t get ourselves into trouble by doing a release before anyone\u0027s ever acquired, thereby letting *two* threads acquire at once :-)","commit_id":"6d160797fc3257942618a7914d526911ebbda328"}],"test/unit/common/test_utils.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8c67f5bda4e1d4e3d9e32524bf8c6b59d20f8ef4","unresolved":false,"context_lines":[{"line_number":6284,"context_line":""},{"line_number":6285,"context_line":"    def test_recursive(self):"},{"line_number":6286,"context_line":"        self.assertTrue(self.mutex.acquire(blocking\u003dFalse))"},{"line_number":6287,"context_line":"        self.assertTrue(self.mutex.acquire(blocking\u003dFalse))"},{"line_number":6288,"context_line":""},{"line_number":6289,"context_line":"    def test_blocking(self):"},{"line_number":6290,"context_line":"        evt \u003d eventlet.event.Event()"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_272d8ea7","line":6287,"updated":"2017-08-16 15:46:53.000000000","message":"should probably also test that the recursion depth unwinds correctly as the mutext is released","commit_id":"3187ce04d6ba78380e9046673dd47f9d6aebdad6"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c0646370c4eff06d64546031f657ada7f59df2a4","unresolved":false,"context_lines":[{"line_number":6370,"context_line":"            pthread2_event.set()"},{"line_number":6371,"context_line":""},{"line_number":6372,"context_line":"        def pthread2():"},{"line_number":6373,"context_line":"            self.mutex.acquire()"},{"line_number":6374,"context_line":"            pthread1_event.set()"},{"line_number":6375,"context_line":"            pthread2_event.wait()"},{"line_number":6376,"context_line":"            owner.append(self.mutex.owner)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_3cb47c7f","line":6373,"updated":"2017-08-16 16:22:24.000000000","message":"Theoretically, there\u0027s a chance that pthread2 will acquire the lock first and the test will deadlock.\n\nIn practice, it seems like we\u0027re saved by\n\n- starting pthread1 first and\n- the overhead of creating a Thread object for pthread2\n\n...but it still makes me a bit nervous. Maybe we should just go ahead and roll in https://review.openstack.org/#/c/494242/ ?","commit_id":"3187ce04d6ba78380e9046673dd47f9d6aebdad6"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"da743a81865ae7f0096b357df0808e67e668cdd3","unresolved":false,"context_lines":[{"line_number":6370,"context_line":"            pthread2_event.set()"},{"line_number":6371,"context_line":""},{"line_number":6372,"context_line":"        def pthread2():"},{"line_number":6373,"context_line":"            self.mutex.acquire()"},{"line_number":6374,"context_line":"            pthread1_event.set()"},{"line_number":6375,"context_line":"            pthread2_event.wait()"},{"line_number":6376,"context_line":"            owner.append(self.mutex.owner)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_a1360102","line":6373,"in_reply_to":"9f436f4f_3cb47c7f","updated":"2017-08-16 21:00:06.000000000","message":"Yeah, that\u0027s probably a good idea. It\u0027s almost like writing concurrent code is hard or something. :p","commit_id":"3187ce04d6ba78380e9046673dd47f9d6aebdad6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8c67f5bda4e1d4e3d9e32524bf8c6b59d20f8ef4","unresolved":false,"context_lines":[{"line_number":6386,"context_line":""},{"line_number":6387,"context_line":"        real_thread1.join()"},{"line_number":6388,"context_line":"        real_thread2.join()"},{"line_number":6389,"context_line":"        self.assertIsNotNone(owner[0])"},{"line_number":6390,"context_line":""},{"line_number":6391,"context_line":"    @classmethod"},{"line_number":6392,"context_line":"    def tearDownClass(cls):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_c2e230d5","line":6389,"updated":"2017-08-16 15:46:53.000000000","message":"I couldn\u0027t quite figure out what this test was covering - the only assertion is that the mutex had an owner when pthread2 acquired the lock, but not who the owner is.","commit_id":"3187ce04d6ba78380e9046673dd47f9d6aebdad6"}]}