Skip to content

Commit 74723e1

Browse files
dcollisonvstinner
andauthored
gh-109461: Update logging module lock to use context manager (#109462)
Co-authored-by: Victor Stinner <vstinner@python.org>
1 parent cc54bcf commit 74723e1

File tree

6 files changed

+81
-176
lines changed

6 files changed

+81
-176
lines changed

Lib/logging/__init__.py

+45-91
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,9 @@ def addLevelName(level, levelName):
159159
160160
This is used when converting levels to text during message formatting.
161161
"""
162-
_acquireLock()
163-
try: #unlikely to cause an exception, but you never know...
162+
with _lock:
164163
_levelToName[level] = levelName
165164
_nameToLevel[levelName] = level
166-
finally:
167-
_releaseLock()
168165

169166
if hasattr(sys, "_getframe"):
170167
currentframe = lambda: sys._getframe(1)
@@ -231,25 +228,27 @@ def _checkLevel(level):
231228
#
232229
_lock = threading.RLock()
233230

234-
def _acquireLock():
231+
def _prepareFork():
235232
"""
236-
Acquire the module-level lock for serializing access to shared data.
233+
Prepare to fork a new child process by acquiring the module-level lock.
237234
238-
This should be released with _releaseLock().
235+
This should be used in conjunction with _afterFork().
239236
"""
240-
if _lock:
241-
try:
242-
_lock.acquire()
243-
except BaseException:
244-
_lock.release()
245-
raise
237+
# Wrap the lock acquisition in a try-except to prevent the lock from being
238+
# abandoned in the event of an asynchronous exception. See gh-106238.
239+
try:
240+
_lock.acquire()
241+
except BaseException:
242+
_lock.release()
243+
raise
246244

247-
def _releaseLock():
245+
def _afterFork():
248246
"""
249-
Release the module-level lock acquired by calling _acquireLock().
247+
After a new child process has been forked, release the module-level lock.
248+
249+
This should be used in conjunction with _prepareFork().
250250
"""
251-
if _lock:
252-
_lock.release()
251+
_lock.release()
253252

254253

255254
# Prevent a held logging lock from blocking a child from logging.
@@ -264,23 +263,20 @@ def _register_at_fork_reinit_lock(instance):
264263
_at_fork_reinit_lock_weakset = weakref.WeakSet()
265264

266265
def _register_at_fork_reinit_lock(instance):
267-
_acquireLock()
268-
try:
266+
with _lock:
269267
_at_fork_reinit_lock_weakset.add(instance)
270-
finally:
271-
_releaseLock()
272268

273269
def _after_at_fork_child_reinit_locks():
274270
for handler in _at_fork_reinit_lock_weakset:
275271
handler._at_fork_reinit()
276272

277-
# _acquireLock() was called in the parent before forking.
273+
# _prepareFork() was called in the parent before forking.
278274
# The lock is reinitialized to unlocked state.
279275
_lock._at_fork_reinit()
280276

281-
os.register_at_fork(before=_acquireLock,
277+
os.register_at_fork(before=_prepareFork,
282278
after_in_child=_after_at_fork_child_reinit_locks,
283-
after_in_parent=_releaseLock)
279+
after_in_parent=_afterFork)
284280

285281

286282
#---------------------------------------------------------------------------
@@ -883,25 +879,20 @@ def _removeHandlerRef(wr):
883879
# set to None. It can also be called from another thread. So we need to
884880
# pre-emptively grab the necessary globals and check if they're None,
885881
# to prevent race conditions and failures during interpreter shutdown.
886-
acquire, release, handlers = _acquireLock, _releaseLock, _handlerList
887-
if acquire and release and handlers:
888-
acquire()
889-
try:
890-
handlers.remove(wr)
891-
except ValueError:
892-
pass
893-
finally:
894-
release()
882+
handlers, lock = _handlerList, _lock
883+
if lock and handlers:
884+
with lock:
885+
try:
886+
handlers.remove(wr)
887+
except ValueError:
888+
pass
895889

896890
def _addHandlerRef(handler):
897891
"""
898892
Add a handler to the internal cleanup list using a weak reference.
899893
"""
900-
_acquireLock()
901-
try:
894+
with _lock:
902895
_handlerList.append(weakref.ref(handler, _removeHandlerRef))
903-
finally:
904-
_releaseLock()
905896

906897

907898
def getHandlerByName(name):
@@ -946,15 +937,12 @@ def get_name(self):
946937
return self._name
947938

948939
def set_name(self, name):
949-
_acquireLock()
950-
try:
940+
with _lock:
951941
if self._name in _handlers:
952942
del _handlers[self._name]
953943
self._name = name
954944
if name:
955945
_handlers[name] = self
956-
finally:
957-
_releaseLock()
958946

959947
name = property(get_name, set_name)
960948

@@ -1026,11 +1014,8 @@ def handle(self, record):
10261014
if isinstance(rv, LogRecord):
10271015
record = rv
10281016
if rv:
1029-
self.acquire()
1030-
try:
1017+
with self.lock:
10311018
self.emit(record)
1032-
finally:
1033-
self.release()
10341019
return rv
10351020

10361021
def setFormatter(self, fmt):
@@ -1058,13 +1043,10 @@ def close(self):
10581043
methods.
10591044
"""
10601045
#get the module data lock, as we're updating a shared structure.
1061-
_acquireLock()
1062-
try: #unlikely to raise an exception, but you never know...
1046+
with _lock:
10631047
self._closed = True
10641048
if self._name and self._name in _handlers:
10651049
del _handlers[self._name]
1066-
finally:
1067-
_releaseLock()
10681050

10691051
def handleError(self, record):
10701052
"""
@@ -1141,12 +1123,9 @@ def flush(self):
11411123
"""
11421124
Flushes the stream.
11431125
"""
1144-
self.acquire()
1145-
try:
1126+
with self.lock:
11461127
if self.stream and hasattr(self.stream, "flush"):
11471128
self.stream.flush()
1148-
finally:
1149-
self.release()
11501129

11511130
def emit(self, record):
11521131
"""
@@ -1182,12 +1161,9 @@ def setStream(self, stream):
11821161
result = None
11831162
else:
11841163
result = self.stream
1185-
self.acquire()
1186-
try:
1164+
with self.lock:
11871165
self.flush()
11881166
self.stream = stream
1189-
finally:
1190-
self.release()
11911167
return result
11921168

11931169
def __repr__(self):
@@ -1237,8 +1213,7 @@ def close(self):
12371213
"""
12381214
Closes the stream.
12391215
"""
1240-
self.acquire()
1241-
try:
1216+
with self.lock:
12421217
try:
12431218
if self.stream:
12441219
try:
@@ -1254,8 +1229,6 @@ def close(self):
12541229
# Also see Issue #42378: we also rely on
12551230
# self._closed being set to True there
12561231
StreamHandler.close(self)
1257-
finally:
1258-
self.release()
12591232

12601233
def _open(self):
12611234
"""
@@ -1391,8 +1364,7 @@ def getLogger(self, name):
13911364
rv = None
13921365
if not isinstance(name, str):
13931366
raise TypeError('A logger name must be a string')
1394-
_acquireLock()
1395-
try:
1367+
with _lock:
13961368
if name in self.loggerDict:
13971369
rv = self.loggerDict[name]
13981370
if isinstance(rv, PlaceHolder):
@@ -1407,8 +1379,6 @@ def getLogger(self, name):
14071379
rv.manager = self
14081380
self.loggerDict[name] = rv
14091381
self._fixupParents(rv)
1410-
finally:
1411-
_releaseLock()
14121382
return rv
14131383

14141384
def setLoggerClass(self, klass):
@@ -1471,12 +1441,11 @@ def _clear_cache(self):
14711441
Called when level changes are made
14721442
"""
14731443

1474-
_acquireLock()
1475-
for logger in self.loggerDict.values():
1476-
if isinstance(logger, Logger):
1477-
logger._cache.clear()
1478-
self.root._cache.clear()
1479-
_releaseLock()
1444+
with _lock:
1445+
for logger in self.loggerDict.values():
1446+
if isinstance(logger, Logger):
1447+
logger._cache.clear()
1448+
self.root._cache.clear()
14801449

14811450
#---------------------------------------------------------------------------
14821451
# Logger classes and functions
@@ -1701,23 +1670,17 @@ def addHandler(self, hdlr):
17011670
"""
17021671
Add the specified handler to this logger.
17031672
"""
1704-
_acquireLock()
1705-
try:
1673+
with _lock:
17061674
if not (hdlr in self.handlers):
17071675
self.handlers.append(hdlr)
1708-
finally:
1709-
_releaseLock()
17101676

17111677
def removeHandler(self, hdlr):
17121678
"""
17131679
Remove the specified handler from this logger.
17141680
"""
1715-
_acquireLock()
1716-
try:
1681+
with _lock:
17171682
if hdlr in self.handlers:
17181683
self.handlers.remove(hdlr)
1719-
finally:
1720-
_releaseLock()
17211684

17221685
def hasHandlers(self):
17231686
"""
@@ -1795,16 +1758,13 @@ def isEnabledFor(self, level):
17951758
try:
17961759
return self._cache[level]
17971760
except KeyError:
1798-
_acquireLock()
1799-
try:
1761+
with _lock:
18001762
if self.manager.disable >= level:
18011763
is_enabled = self._cache[level] = False
18021764
else:
18031765
is_enabled = self._cache[level] = (
18041766
level >= self.getEffectiveLevel()
18051767
)
1806-
finally:
1807-
_releaseLock()
18081768
return is_enabled
18091769

18101770
def getChild(self, suffix):
@@ -1834,16 +1794,13 @@ def _hierlevel(logger):
18341794
return 1 + logger.name.count('.')
18351795

18361796
d = self.manager.loggerDict
1837-
_acquireLock()
1838-
try:
1797+
with _lock:
18391798
# exclude PlaceHolders - the last check is to ensure that lower-level
18401799
# descendants aren't returned - if there are placeholders, a logger's
18411800
# parent field might point to a grandparent or ancestor thereof.
18421801
return set(item for item in d.values()
18431802
if isinstance(item, Logger) and item.parent is self and
18441803
_hierlevel(item) == 1 + _hierlevel(item.parent))
1845-
finally:
1846-
_releaseLock()
18471804

18481805
def __repr__(self):
18491806
level = getLevelName(self.getEffectiveLevel())
@@ -2102,8 +2059,7 @@ def basicConfig(**kwargs):
21022059
"""
21032060
# Add thread safety in case someone mistakenly calls
21042061
# basicConfig() from multiple threads
2105-
_acquireLock()
2106-
try:
2062+
with _lock:
21072063
force = kwargs.pop('force', False)
21082064
encoding = kwargs.pop('encoding', None)
21092065
errors = kwargs.pop('errors', 'backslashreplace')
@@ -2152,8 +2108,6 @@ def basicConfig(**kwargs):
21522108
if kwargs:
21532109
keys = ', '.join(kwargs.keys())
21542110
raise ValueError('Unrecognised argument(s): %s' % keys)
2155-
finally:
2156-
_releaseLock()
21572111

21582112
#---------------------------------------------------------------------------
21592113
# Utility functions at module level.

0 commit comments

Comments
 (0)