Skip to content

Commit 87efddd

Browse files
committed
Fix deadlock bug
1 parent 6ed676a commit 87efddd

File tree

2 files changed

+31
-1
lines changed

2 files changed

+31
-1
lines changed

opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,7 @@ def __init__(
489489
) -> None:
490490
super().__init__(level=level)
491491
self._logger_provider = logger_provider or get_logger_provider()
492+
# self.flushOnClose = False
492493

493494
@staticmethod
494495
def _get_attributes(record: logging.LogRecord) -> _ExtendedAttributes:
@@ -584,7 +585,8 @@ def flush(self) -> None:
584585
if hasattr(self._logger_provider, "force_flush") and callable(
585586
self._logger_provider.force_flush
586587
):
587-
self._logger_provider.force_flush()
588+
thread = threading.Thread(target=self._logger_provider.force_flush)
589+
thread.start()
588590

589591

590592
class Logger(APILogger):

opentelemetry-sdk/tests/logs/test_handler.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,34 @@ def test_handler_custom_log_level(self):
5959
logger.critical("No Time For Caution")
6060
self.assertEqual(processor.emit_count(), 2)
6161

62+
def test_handler_calling_flush_does_not_cause_deadlock(self):
63+
class LogProcessorThatAccessesLockOnFlush(LogRecordProcessor):
64+
def emit(self, log_data: LogData):
65+
pass
66+
67+
def shutdown(self):
68+
pass
69+
70+
def force_flush(self, timeout_millis: int = 30000):
71+
# Deadlock will happen here IF `flush` starts a new thread
72+
# and then blocks, if it just starts a thread and then returns
73+
# we don't seem to encounter the issue..
74+
logging._lock.acquire()
75+
# assert logging._lock.acquire(False) is True
76+
logging._lock.release()
77+
78+
logger_provider = LoggerProvider()
79+
processor = LogProcessorThatAccessesLockOnFlush()
80+
logger_provider.add_log_record_processor(processor)
81+
handler = LoggingHandler(logger_provider=logger_provider)
82+
logging.getLogger().addHandler(handler)
83+
# The below code is essentially recreating what is causing the problem inside
84+
# logging.config.dictConfig. Actually calling logging.config.dictConfig will modify
85+
# global state inside the logging module and break lots of tests.
86+
with logging._lock:
87+
for handler in logging.getLogger().handlers:
88+
handler.flush()
89+
6290
# pylint: disable=protected-access
6391
def test_log_record_emit_noop(self):
6492
noop_logger_provder = NoOpLoggerProvider()

0 commit comments

Comments
 (0)