thread safe MultiprocessingLoggingHandler

[Tobias314]

Description:

• Improves the MultiprocessExecutor logging by making it more thread safe and allowing thread-specific log handlers
• Required for https://github.com/scalableminds/voxelytics/pull/4555

Todos:

• Updated Changelog

[gemini-code-assist]

Code Review

This pull request introduces thread-safety for logging in a multiprocessing context by implementing a global lock (ROOT_LOGGER_LOCK) for root logger modifications and adding checks for thread-owned handlers via _owner_thread_id. Review feedback identifies significant issues in the decrement_usage method, including a non-atomic counter, a race condition during handler closure, and the risk of closing global handlers prematurely. A suggestion is provided to move these operations within the lock to ensure consistency.

[gemini-code-assist]

The decrement_usage method has several thread-safety and lifecycle issues:

1. Non-atomic counter: The decrement and check of self._usage_counter (lines 91-92) are not synchronized. If shutdown() is called concurrently on the same executor from different threads, the counter could end up in an inconsistent state.
2. Race condition on close: The wrapped_handler.close() call (line 105) happens outside the ROOT_LOGGER_LOCK. A new pool could be initialized in another thread, see the wrapped_handler (which was added back to the root logger at line 97), and wrap it before it is closed here. This would leave the new pool with a closed and broken handler.
3. Global handler lifecycle: Closing handlers that don't have an _owner_thread_id (lines 104-105) will destroy global handlers (like a standard StreamHandler on the root logger) once the first executor finishes. This breaks logging for the rest of the application.

It is recommended to move the counter logic and the restoration/closure of the handler inside the ROOT_LOGGER_LOCK. Also, consider if the pool should be closing these handlers at all, as they were not created by the pool.

Suggested change

	self._usage_counter -= 1
	if self._usage_counter == 0:
	# unwrap inner handler:
	root_logger = getLogger()
	root_logger.removeHandler(self)
	root_logger.addHandler(self.wrapped_handler)
	with ROOT_LOGGER_LOCK:
	root_logger = getLogger()
	root_logger.removeHandler(self)
	root_logger.addHandler(self.wrapped_handler)
	self._is_closed = True
	self._queue_thread.join(30)
	self._manager.shutdown()
	self.wrapped_handler.close()
	# Thread-owned handlers (e.g. task-specific file handlers) are closed by their
	# owner (attach_logging_handler), not by the pool.
	if not getattr(self.wrapped_handler, "_owner_thread_id", None):
	self.wrapped_handler.close()
	super().close()
	with ROOT_LOGGER_LOCK:
	self._usage_counter -= 1
	if self._usage_counter == 0:
	# unwrap inner handler:
	root_logger = getLogger()
	root_logger.removeHandler(self)
	root_logger.addHandler(self.wrapped_handler)
	# Thread-owned handlers (e.g. task-specific file handlers) are closed by their
	# owner (attach_logging_handler), not by the pool.
	if not getattr(self.wrapped_handler, "_owner_thread_id", None):
	self.wrapped_handler.close()
	if self._usage_counter == 0:
	self._is_closed = True
	self._queue_thread.join(30)
	self._manager.shutdown()
	super().close()

[github-actions]

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines	Covered	Coverage	Threshold	Status
11426	9671	85%	80%	🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: d41223c by action🐍