Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
gh-109461: Update logging module lock acquisition to use context manager#109462
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
Merged
vstinner
merged 41 commits into
python:main
from
dcollison:gh-109461-update-logging-lock-acquisitionSep 27, 2023
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
02c1d58
Updated logging __init__.py to acquire its module lock using a contex…
dcollison f319ab0
📜🤖 Added by blurb_it.
blurb-it[bot] 6664c4c
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
dcollison fd635b2
Updated to leave the existing API alone and added a new method _acqui…
dcollison 749b678
Merge remote-tracking branch 'origin/gh-109461-update-logging-lock-ac…
dcollison e5ad43b
Updated acquireLock usages to acquireModuleLock
dcollison 0a2bb5f
Removed unnecessary changes
dcollison ea33592
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
dcollison af9f7d1
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
dcollison 4f2d1c1
Updated _acquireModuleLock() to _get_lock() which gets the lock's con…
dcollison dc7df1e
Merge remote-tracking branch 'origin/gh-109461-update-logging-lock-ac…
dcollison 892a5cb
Updated comments
dcollison a30d5da
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
dcollison cf7a2e6
Updated blurb
dcollison 0a53f3b
Merge remote-tracking branch 'origin/gh-109461-update-logging-lock-ac…
dcollison 2760cc5
Removed unused import
dcollison b5e6651
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
dcollison 7f3b28a
Updated __init__.py to remove _get_lock() and "if _lock" checks since…
dcollison 09c9133
Changed "with logging._get_lock():" to "with logging._lock:"
dcollison 9f3607e
Changed "with logging._get_lock()" to "with logging._lock:"
dcollison b15e4bf
Changed "with logging._get_lock():" to "with logging._lock:"
dcollison a0c50bf
Update __init__.py
dcollison 9d0675b
Update Misc/NEWS.d/next/Library/2023-09-15-17-12-53.gh-issue-109461.V…
dcollison a298a7b
Update __init__.py
dcollison 1b14732
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
dcollison 1979a75
Updated handlers to use a context manager for lock acquisition
dcollison 3fd33ef
Updated handlers to use a context manager for lock acquisition
dcollison 22c868e
Copying _lock to local variable
dcollison 335f814
Updated another lock acquisition
dcollison afa79ef
Update Misc/NEWS.d/next/Library/2023-09-15-17-12-53.gh-issue-109461.V…
dcollison 600a126
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
dcollison 5e92580
Someone managed to duplicate a line in the docstring
dcollison 20e46f6
Merge remote-tracking branch 'origin/gh-109461-update-logging-lock-ac…
dcollison c73e2b7
Someone managed to duplicate a line in the docstring
dcollison a9472c7
Reverted Handler acquire() and release() as it actually needs the Non…
dcollison df75801
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
dcollison bca5ae8
Added comment to _acquireLock() to justify the try-except block aroun…
dcollison 1ba5979
Renamed "_acquireLock" to "_prepareFork" and "_releaseLock" to "_afte…
dcollison c1a9651
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
dcollison efd7462
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
dcollison 797ae0d
Merge branch 'main' into gh-109461-update-logging-lock-acquisition
AA-Turner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -159,12 +159,9 @@ def addLevelName(level, levelName): | ||
| This is used when converting levels to text during message formatting. | ||
| """ | ||
| _acquireLock() | ||
| try: #unlikely to cause an exception, but you never know... | ||
| with _lock: | ||
| _levelToName[level] = levelName | ||
| _nameToLevel[levelName] = level | ||
| finally: | ||
| _releaseLock() | ||
| if hasattr(sys, "_getframe"): | ||
| currentframe = lambda: sys._getframe(1) | ||
| @@ -231,25 +228,27 @@ def _checkLevel(level): | ||
| # | ||
| _lock = threading.RLock() | ||
| def _acquireLock(): | ||
| def _prepareFork(): | ||
| """ | ||
| Acquire the module-level lock for serializing access to shared data. | ||
| Prepare to fork a new child process by acquiring the module-level lock. | ||
| This should be released with _releaseLock(). | ||
| This should be used in conjunction with _afterFork(). | ||
| """ | ||
| if _lock: | ||
| try: | ||
| _lock.acquire() | ||
| except BaseException: | ||
| _lock.release() | ||
| raise | ||
| # Wrap the lock acquisition in a try-except to prevent the lock from being | ||
| # abandoned in the event of an asynchronous exception. See gh-106238. | ||
| try: | ||
| _lock.acquire() | ||
| except BaseException: | ||
| _lock.release() | ||
| raise | ||
| def _releaseLock(): | ||
| def _afterFork(): | ||
| """ | ||
vstinner marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| Release the module-level lock acquired by calling _acquireLock(). | ||
| After a new child process has been forked, release the module-level lock. | ||
| This should be used in conjunction with _prepareFork(). | ||
| """ | ||
| if _lock: | ||
| _lock.release() | ||
| _lock.release() | ||
| # Prevent a held logging lock from blocking a child from logging. | ||
| @@ -264,23 +263,20 @@ def _register_at_fork_reinit_lock(instance): | ||
| _at_fork_reinit_lock_weakset = weakref.WeakSet() | ||
| def _register_at_fork_reinit_lock(instance): | ||
| _acquireLock() | ||
| try: | ||
| with _lock: | ||
| _at_fork_reinit_lock_weakset.add(instance) | ||
| finally: | ||
| _releaseLock() | ||
| def _after_at_fork_child_reinit_locks(): | ||
| for handler in _at_fork_reinit_lock_weakset: | ||
| handler._at_fork_reinit() | ||
| # _acquireLock() was called in the parent before forking. | ||
| # _prepareFork() was called in the parent before forking. | ||
| # The lock is reinitialized to unlocked state. | ||
| _lock._at_fork_reinit() | ||
| os.register_at_fork(before=_acquireLock, | ||
| os.register_at_fork(before=_prepareFork, | ||
| after_in_child=_after_at_fork_child_reinit_locks, | ||
| after_in_parent=_releaseLock) | ||
| after_in_parent=_afterFork) | ||
| #--------------------------------------------------------------------------- | ||
| @@ -883,25 +879,20 @@ def _removeHandlerRef(wr): | ||
| # set to None. It can also be called from another thread. So we need to | ||
| # pre-emptively grab the necessary globals and check if they're None, | ||
| # to prevent race conditions and failures during interpreter shutdown. | ||
| acquire, release, handlers = _acquireLock, _releaseLock, _handlerList | ||
| if acquire and release and handlers: | ||
| acquire() | ||
| try: | ||
| handlers.remove(wr) | ||
| except ValueError: | ||
| pass | ||
| finally: | ||
| release() | ||
| handlers, lock = _handlerList, _lock | ||
| if lock and handlers: | ||
| with lock: | ||
| try: | ||
| handlers.remove(wr) | ||
| except ValueError: | ||
| pass | ||
| def _addHandlerRef(handler): | ||
| """ | ||
| Add a handler to the internal cleanup list using a weak reference. | ||
| """ | ||
| _acquireLock() | ||
| try: | ||
| with _lock: | ||
| _handlerList.append(weakref.ref(handler, _removeHandlerRef)) | ||
| finally: | ||
| _releaseLock() | ||
| def getHandlerByName(name): | ||
| @@ -946,15 +937,12 @@ def get_name(self): | ||
| return self._name | ||
| def set_name(self, name): | ||
| _acquireLock() | ||
| try: | ||
| with _lock: | ||
| if self._name in _handlers: | ||
| del _handlers[self._name] | ||
| self._name = name | ||
| if name: | ||
| _handlers[name] = self | ||
| finally: | ||
| _releaseLock() | ||
| name = property(get_name, set_name) | ||
| @@ -1026,11 +1014,8 @@ def handle(self, record): | ||
| if isinstance(rv, LogRecord): | ||
| record = rv | ||
| if rv: | ||
| self.acquire() | ||
| try: | ||
| with self.lock: | ||
| self.emit(record) | ||
| finally: | ||
| self.release() | ||
| return rv | ||
| def setFormatter(self, fmt): | ||
| @@ -1058,13 +1043,10 @@ def close(self): | ||
| methods. | ||
| """ | ||
| #get the module data lock, as we're updating a shared structure. | ||
| _acquireLock() | ||
| try: #unlikely to raise an exception, but you never know... | ||
| with _lock: | ||
| self._closed = True | ||
| if self._name and self._name in _handlers: | ||
| del _handlers[self._name] | ||
| finally: | ||
| _releaseLock() | ||
| def handleError(self, record): | ||
| """ | ||
| @@ -1141,12 +1123,9 @@ def flush(self): | ||
| """ | ||
| Flushes the stream. | ||
| """ | ||
| self.acquire() | ||
| try: | ||
| with self.lock: | ||
| if self.stream and hasattr(self.stream, "flush"): | ||
| self.stream.flush() | ||
| finally: | ||
| self.release() | ||
| def emit(self, record): | ||
| """ | ||
| @@ -1182,12 +1161,9 @@ def setStream(self, stream): | ||
| result = None | ||
| else: | ||
| result = self.stream | ||
| self.acquire() | ||
| try: | ||
| with self.lock: | ||
| self.flush() | ||
| self.stream = stream | ||
| finally: | ||
| self.release() | ||
| return result | ||
| def __repr__(self): | ||
| @@ -1237,8 +1213,7 @@ def close(self): | ||
| """ | ||
| Closes the stream. | ||
| """ | ||
| self.acquire() | ||
| try: | ||
| with self.lock: | ||
| try: | ||
| if self.stream: | ||
| try: | ||
| @@ -1254,8 +1229,6 @@ def close(self): | ||
| # Also see Issue #42378: we also rely on | ||
| # self._closed being set to True there | ||
| StreamHandler.close(self) | ||
| finally: | ||
| self.release() | ||
| def _open(self): | ||
| """ | ||
| @@ -1391,8 +1364,7 @@ def getLogger(self, name): | ||
| rv = None | ||
| if not isinstance(name, str): | ||
| raise TypeError('A logger name must be a string') | ||
| _acquireLock() | ||
| try: | ||
| with _lock: | ||
| if name in self.loggerDict: | ||
| rv = self.loggerDict[name] | ||
| if isinstance(rv, PlaceHolder): | ||
| @@ -1407,8 +1379,6 @@ def getLogger(self, name): | ||
| rv.manager = self | ||
| self.loggerDict[name] = rv | ||
| self._fixupParents(rv) | ||
| finally: | ||
| _releaseLock() | ||
| return rv | ||
| def setLoggerClass(self, klass): | ||
| @@ -1471,12 +1441,11 @@ def _clear_cache(self): | ||
| Called when level changes are made | ||
| """ | ||
| _acquireLock() | ||
| for logger in self.loggerDict.values(): | ||
| if isinstance(logger, Logger): | ||
| logger._cache.clear() | ||
| self.root._cache.clear() | ||
| _releaseLock() | ||
| with _lock: | ||
| for logger in self.loggerDict.values(): | ||
| if isinstance(logger, Logger): | ||
| logger._cache.clear() | ||
| self.root._cache.clear() | ||
| #--------------------------------------------------------------------------- | ||
| # Logger classes and functions | ||
| @@ -1701,23 +1670,17 @@ def addHandler(self, hdlr): | ||
| """ | ||
| Add the specified handler to this logger. | ||
| """ | ||
| _acquireLock() | ||
| try: | ||
| with _lock: | ||
| if not (hdlr in self.handlers): | ||
| self.handlers.append(hdlr) | ||
| finally: | ||
| _releaseLock() | ||
| def removeHandler(self, hdlr): | ||
| """ | ||
| Remove the specified handler from this logger. | ||
| """ | ||
| _acquireLock() | ||
| try: | ||
| with _lock: | ||
| if hdlr in self.handlers: | ||
| self.handlers.remove(hdlr) | ||
| finally: | ||
| _releaseLock() | ||
| def hasHandlers(self): | ||
| """ | ||
| @@ -1795,16 +1758,13 @@ def isEnabledFor(self, level): | ||
| try: | ||
| return self._cache[level] | ||
| except KeyError: | ||
| _acquireLock() | ||
| try: | ||
| with _lock: | ||
| if self.manager.disable >= level: | ||
| is_enabled = self._cache[level] = False | ||
| else: | ||
| is_enabled = self._cache[level] = ( | ||
| level >= self.getEffectiveLevel() | ||
| ) | ||
| finally: | ||
| _releaseLock() | ||
| return is_enabled | ||
| def getChild(self, suffix): | ||
| @@ -1834,16 +1794,13 @@ def _hierlevel(logger): | ||
| return 1 + logger.name.count('.') | ||
| d = self.manager.loggerDict | ||
| _acquireLock() | ||
| try: | ||
| with _lock: | ||
| # exclude PlaceHolders - the last check is to ensure that lower-level | ||
| # descendants aren't returned - if there are placeholders, a logger's | ||
| # parent field might point to a grandparent or ancestor thereof. | ||
| return set(item for item in d.values() | ||
| if isinstance(item, Logger) and item.parent is self and | ||
| _hierlevel(item) == 1 + _hierlevel(item.parent)) | ||
| finally: | ||
| _releaseLock() | ||
| def __repr__(self): | ||
| level = getLevelName(self.getEffectiveLevel()) | ||
| @@ -2102,8 +2059,7 @@ def basicConfig(**kwargs): | ||
| """ | ||
| # Add thread safety in case someone mistakenly calls | ||
| # basicConfig() from multiple threads | ||
| _acquireLock() | ||
| try: | ||
| with _lock: | ||
| force = kwargs.pop('force', False) | ||
| encoding = kwargs.pop('encoding', None) | ||
| errors = kwargs.pop('errors', 'backslashreplace') | ||
| @@ -2152,8 +2108,6 @@ def basicConfig(**kwargs): | ||
| if kwargs: | ||
| keys = ', '.join(kwargs.keys()) | ||
| raise ValueError('Unrecognised argument(s): %s' % keys) | ||
| finally: | ||
| _releaseLock() | ||
| #--------------------------------------------------------------------------- | ||
| # Utility functions at module level. | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.