Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update logging library module lock to use context manager to acquire/release lock. #109461

Closed
dcollison opened this issue Sep 15, 2023 · 0 comments
Labels
type-feature A feature request or enhancement

Comments

@dcollison
Copy link
Contributor

dcollison commented Sep 15, 2023

Feature or enhancement

Proposal:

Current implementation relies on both _acquireLock() and _releaseLock() being called, otherwise a lock may never be released:

def _acquireLock():
    """
    Acquire the module-level lock for serializing access to shared data.

    This should be released with _releaseLock().
    """
    if _lock:
        try:
            _lock.acquire()
        except BaseException:
            _lock.release()
            raise

def _releaseLock():
    """
    Release the module-level lock acquired by calling _acquireLock().
    """
    if _lock:
        _lock.release()

The majority of usages of _acquireLock() manually add a try/except/finally block to ensure that the lock is released if an exception is thrown. Some usages of _acquireLock() have no safety.

The proposal is to alter the usage of _acquireLock() to be a context manager that deals with acquiring and releasing automatically rather than requiring try/except/finally blocks to be used anywhere the function is called.

For example,
usage before:

_acquireLock()
try:
    ...
finally:
    _releaseLock()

proposed usage:

with _acquireLock():
    ...

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

@dcollison dcollison added the type-feature A feature request or enhancement label Sep 15, 2023
dcollison added a commit to dcollison/cpython that referenced this issue Sep 15, 2023
…ock-acquisition' into pythongh-109461-update-logging-lock-acquisition
dcollison added a commit to dcollison/cpython that referenced this issue Sep 16, 2023
…ock-acquisition' into pythongh-109461-update-logging-lock-acquisition
dcollison added a commit to dcollison/cpython that referenced this issue Sep 16, 2023
…ock-acquisition' into pythongh-109461-update-logging-lock-acquisition
dcollison added a commit to dcollison/cpython that referenced this issue Sep 21, 2023
…ock-acquisition' into pythongh-109461-update-logging-lock-acquisition
vstinner added a commit that referenced this issue Sep 27, 2023
Co-authored-by: Victor Stinner <vstinner@python.org>
csm10495 pushed a commit to csm10495/cpython that referenced this issue Sep 28, 2023
The-Compiler added a commit to qutebrowser/qutebrowser that referenced this issue Jun 25, 2024
See python/cpython#109461
Using it as a context manager already works fine in earlier Python versions too.

See #8205
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

1 participant