gh-106238: Handle KeyboardInterrupt during logging._acquireLock() #106239
Conversation
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. |
|
All commit authors signed the Contributor License Agreement. |
AlexWaygood
changed the title
|
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 |
|
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. |
|
@arieleiz You need to sign the CLA before this PR can be progressed. |
|
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? |
|
I'm afraid not. Might be worth asking on https://discuss.python.org/ to see if anyone there can help. |
vsajip
added
the
🔨 test-with-buildbots
bedevere-bot
removed
the
🔨 test-with-buildbots
|
CLA signing status appears to be stuck on "not signed". Will convert to draft and back to see if it unsticks. |
That didn't work - closing and reopening to see if that does the trick. |
|
Thanks @vsajip! shows up as CLA signed for me |
Misc/NEWS.d/next/Library/2023-06-29-12-40-52.gh-issue-106238.VulKb9.rst
Outdated
Show resolved
Hide resolved
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.