Repository navigation
gh-106238: Handle KeyboardInterrupt during logging._acquireLock() - #106239
vsajip merged 3 commits into
Conversation
Sorry, something went wrong.
We've come across a concurrency bug in logging/__init__.py which involves the handling of asynchronous exceptions, such as KeyboardInterrupt, during the execution of logging._acquireLock().
In the current implementation, when threading.RLock.acquire() is executed, there is a possibility for an asynchronous exception to occur during the transition back from native code, even if the lock acquisition is successful.
The typical use of _acquireLock() in the logging library is as follows:
def _loggingMethod(handler):
"""
Add a handler to the internal cleanup list using a weak reference.
"""
_acquireLock()
try:
# doSomething
finally:
_releaseLock()
In this pattern, if a KeyboardInterrupt is raised during the lock acquisition, the lock ends up getting abandoned.
When can this happen? One example is during forks. logging/__init__.py registers an at-fork hook, with
os.register_at_fork(before=_acquireLock,
after_in_child=_after_at_fork_child_reinit_locks,
after_in_parent=_releaseLock)
A scenario occurring in our production environment is during a slow fork operation (when the server is under heavy load and performing a multitude of forks). The lock could be held for up to a minute. If this is happening in a secondary thread, and a SIGINT signal is received in the main thread while is waiting to acquire the lock for logging, the lock will be abandoned. This will causes the process to hang during the next _acquireLock() call.
To address this issue, we provide a simple pull request to add a try-except block within _acquireLock(), e.g.:
def _acquireLock():
if _lock:
try:
_lock.acquire()
except BaseException:
_lock.release()
raise
This way, if an exception arises during the lock acquisition, the lock will be released, preventing the lock from being abandoned and the process from potentially hanging.
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
Sorry, something went wrong.
|
All commit authors signed the Contributor License Agreement. |
Sorry, something went wrong.
|
I'm not sure this is the right place to fix this. See the long discussion on #50970. Anyway, wouldn't your proposed fix be better arranged to move the lock.acquire() to inside the try:? That way, the finally: would release the lock. |
Sorry, something went wrong.
|
Thank you very much for pointing me to #50970. It is not the same issue but similar, as we encounter the issue in the original process, not the forked process, and the issue is not the state of the lock but the lock being abandoned due to an async exception. |
Sorry, something went wrong.
|
@arieleiz You need to sign the CLA before this PR can be progressed. |
Sorry, something went wrong.
|
Hi @vsajip, I've tried signing the CLA multiple times, I keep getting a "Internal Server Error" from CLAbot after "Sign here with github to agree". Any idea what I am doing wrong? |
Sorry, something went wrong.
|
I'm afraid not. Might be worth asking on https://discuss.python.org/ to see if anyone there can help. |
Sorry, something went wrong.
|
CLA signing status appears to be stuck on "not signed". Will convert to draft and back to see if it unsticks. |
Sorry, something went wrong.
That didn't work - closing and reopening to see if that does the trick. |
Sorry, something went wrong.
|
Thanks @vsajip! shows up as CLA signed for me |
Sorry, something went wrong.
We've come across a concurrency bug in logging/init.py which involves the handling of asynchronous exceptions, such as KeyboardInterrupt, during the execution of logging._acquireLock().
In the current implementation, when threading.RLock.acquire() is executed, there is a possibility for an asynchronous exception to occur during the transition back from native code, even if the lock acquisition is successful.
The typical use of _acquireLock() in the logging library is as follows:
In this pattern, if a KeyboardInterrupt is raised during the lock acquisition, the lock ends up getting abandoned.
When can this happen? One example is during forks. logging/init.py registers an at-fork hook, with
A scenario occurring in our production environment is during a slow fork operation (when the server is under heavy load and performing a multitude of forks). The lock could be held for up to a minute. If this is happening in a secondary thread, and a SIGINT signal is received in the main thread while is waiting to acquire the lock for logging, the lock will be abandoned. This will causes the process to hang during the next _acquireLock() call.
To address this issue, we provide a simple pull request to add a try-except block within _acquireLock(), e.g.:
This way, if an exception arises during the lock acquisition, the lock will be released, preventing the lock from being abandoned and the process from potentially hanging.