Skip to content

Commit

Permalink
Prevent deadlock on logging._lock
Browse files Browse the repository at this point in the history
Or at least, reduce the likelihood of it.

We've seen deadlocks when GC runs while in the process of releasing an
RLock; it results in a stack that looks like

  File ".../logging/__init__.py", line 232, in _releaseLock
    _lock.release()
  File ".../threading.py", line 189, in release
    self._block.release()
  File ".../eventlet/lock.py", line 24, in release
    return super(Lock, self).release(blocking=blocking)
  File ".../logging/__init__.py", line 831, in _removeHandlerRef
    acquire()
  File ".../logging/__init__.py", line 225, in _acquireLock
    _lock.acquire()

That is, we try to release the lock, but in between clearing the RLock
ownership information and releasing the underlying Lock, GC runs and
invokes a weakref callback, which in turn tries to acquire the RLock.
Since the ownership information has already been cleared, the lock's no
longer re-entrant and everything seizes.

This seems to have become more of a problem since we separated Lock and
Semaphore; apparently the extra stack frame makes it much more likely
that GC can sneak in during that critical moment. So, inline the release
inside of Lock rather than punting to Semaphore; the implementation is
simple anyway, and hasn't changed for at least 12 years (since Semaphore
was pulled out to its own module).

Closes #742
  • Loading branch information
tipabu committed Jan 7, 2022
1 parent 955be1c commit 1f1f1ce
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion eventlet/lock.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from eventlet import hubs
from eventlet.semaphore import Semaphore


Expand All @@ -21,7 +22,15 @@ def release(self, blocking=True):
if self.counter > 0:
raise RuntimeError("release unlocked lock")

return super(Lock, self).release(blocking=blocking)
# Consciously *do not* call super().release(), but instead inline
# Semaphore.release() here. We've seen issues with logging._lock
# deadlocking because garbage collection happened to run mid-release
# and eliminating the extra stack frame should help prevent that.
# See https://github.com/eventlet/eventlet/issues/742
self.counter += 1
if self._waiters:
hubs.get_hub().schedule_call_global(0, self._do_acquire)
return True

def _at_fork_reinit(self):
self.counter = 1
Expand Down

0 comments on commit 1f1f1ce

Please sign in to comment.