Fix for Runtime error when reinitializing logger

[jeremyk]

We hit this error intermittently as we have to reinitialize our logger at least once during our application startup.

File "/opt/pysetup/.venv/lib/python3.11/site-packages/loguru/_logger.py", line 2031, in _log
for handler in core.handlers.values():
RuntimeError: dictionary changed size during iteration

Fix is to copy the values to avoid the "changed size" error.

Fixes: #1183

[jeremyk]

Bump - hoping this can get a look sometime soon!

[trim21]

this doesn't actually fix the problem, it only make error less possible to happen。

should choose lock, if we think loguru should support logging and reinit at same time.

there is already core.lock, just acquire it

[Delgan]

Thanks for investigating this bug and opening a PR, @jeremyk. Apologies for the delay.

Actually, the logger was supposed to be thread-safe in such scenario. Unfortunately, there is a mistake in the implementation.

The _core.handlers must be copied into a variable intended to be mutated locally.

This is done here for logger.add():

loguru/loguru/_logger.py

Line 1006 in 80f05fd

handlers = self._core.handlers.copy()

And here for logger.remove():

loguru/loguru/_logger.py

Line 1043 in 80f05fd

handlers = self._core.handlers.copy()

Once the dict containing the handlers has been updated as desired, the _core.handlers reference can be replaced with the previously copied one.

This allows the iteration through handlers for logger._log() to be lock-free, @trim21. It's kind of a copy-on-write mechanism. The _log() should acquire a reference to the dict that will never be modified.

Now, I said logger.add() and logger.remove() are actually copying the handlers as expected, but there is still a bug... This is because of that part:

loguru/loguru/_logger.py

Lines 1053 to 1059 in 80f05fd

	for handler_id in handler_ids:
	handler = handlers.pop(handler_id)
	# This needs to be done first in case "stop()" raises an exception
	levelnos = (h.levelno for h in handlers.values())
	self._core.min_level = min(levelnos, default=float("inf"))
	self._core.handlers = handlers

The problem is that after the first iteration, the self._core.handlers is replaced with the locally copied handlers variable. But in subsequent iterations, this variable continues to be modified, while the value is now the same as in _core.handlers. This violates the previously stated principle that handlers dict can only be mutated locally, and that causes RuntimeError: dictionary changed size during iteration now that _log() and remove() are accessing the same dict in parallel.

The fix should be implemented in the remove() method instead of the _log() one. Do you want to update the PR @jeremyk or should I take care of that? It can be a simple as:

iff --git a/loguru/_logger.py b/loguru/_logger.py
index cc514b0..ffe4f80 100644
--- a/loguru/_logger.py
+++ b/loguru/_logger.py
@@ -1040,7 +1040,7 @@ class Logger:
             )
 
         with self._core.lock:
-            handlers = self._core.handlers.copy()
+            handlers = self._core.handlers
 
             if handler_id is not None and handler_id not in handlers:
                 raise ValueError("There is no existing handler with id %d" % handler_id) from None
@@ -1051,6 +1051,7 @@ class Logger:
                 handler_ids = [handler_id]
 
             for handler_id in handler_ids:
+                handlers = handlers.copy()
                 handler = handlers.pop(handler_id)
 
                 # This needs to be done first in case "stop()" raises an exception

[jeremyk]

I'm totally fine if you close this PR and take care of it. Thanks @Delgan !