Fix "remove()" not thread-safe causing possible RuntimeError (#1183)

loguru/_logger.py

@@ -1040,17 +1040,16 @@ def remove(self, handler_id=None):
             )
 
         with self._core.lock:
-            handlers = self._core.handlers.copy()
-
-            if handler_id is not None and handler_id not in handlers:
+            if handler_id is not None and handler_id not in self._core.handlers:
                 raise ValueError("There is no existing handler with id %d" % handler_id) from None
 
             if handler_id is None:
-                handler_ids = list(handlers.keys())
+                handler_ids = list(self._core.handlers)
             else:
                 handler_ids = [handler_id]
 
             for handler_id in handler_ids:
+                handlers = self._core.handlers.copy()
                 handler = handlers.pop(handler_id)
 
                 # This needs to be done first in case "stop()" raises an exception

tests/test_threading.py

@@ -6,8 +6,9 @@
 
 
 class NonSafeSink:
-    def __init__(self, sleep_time):
+    def __init__(self, sleep_time, stop_time=0):
         self.sleep_time = sleep_time
+        self.stop_time = stop_time
         self.written = ""
         self.stopped = False
 
@@ -21,6 +22,7 @@ def write(self, message):
         self.written += message[length:]
 
     def stop(self):
+        time.sleep(self.stop_time)
         self.stopped = True
 
 
@@ -111,6 +113,63 @@ def thread_2():
     assert sink.written == "aaa0bbb\n"
 
 
+def test_safe_removing_all_while_logging(capsys):
+    barrier = Barrier(2)
+
+    for _ in range(1000):
+        logger.add(lambda _: None, format="{message}", catch=False)
+
+    def thread_1():
+        barrier.wait()
+        logger.remove()
+
+    def thread_2():
+        barrier.wait()
+        for _ in range(100):
+            logger.info("Some message")
+
+    threads = [Thread(target=thread_1), Thread(target=thread_2)]
+
+    for thread in threads:
+        thread.start()
+
+    for thread in threads:
+        thread.join()
+
+    out, err = capsys.readouterr()
+    assert out == ""
+    assert err == ""
+
+
+def test_safe_slow_removing_all_while_logging(capsys):
+    barrier = Barrier(2)
+
+    for _ in range(10):
+        sink = NonSafeSink(0.1, 0.1)
+        logger.add(sink, format="{message}", catch=False)
+
+    def thread_1():
+        barrier.wait()
+        logger.remove()
+
+    def thread_2():
+        barrier.wait()
+        time.sleep(0.5)
+        logger.info("Some message")
+
+    threads = [Thread(target=thread_1), Thread(target=thread_2)]
+
+    for thread in threads:
+        thread.start()
+
+    for thread in threads:
+        thread.join()
+
+    out, err = capsys.readouterr()
+    assert out == ""
+    assert err == ""
+
+
 def test_safe_writing_after_removing(capsys):
     barrier = Barrier(2)