GlobalThreadPool singleton: unsynchronised `__new__` and silently ignored `max_workers`

[clarkestu]

backend/utils/thread_utils.py defines a process-wide thread pool used to fan out async work:

class GlobalThreadPool:
    _instance = None

    def __new__(cls, max_workers=10):
        if cls._instance is None:
            cls._instance = super().__new__(cls)
            cls._instance.pool = ThreadPoolExecutor(max_workers=max_workers)
            atexit.register(cls._instance.pool.shutdown)
        return cls._instance

Two distinct problems here:

1. The singleton is not thread-safe. Two threads racing into GlobalThreadPool(...) before _instance is set can both pass the if cls._instance is None check, each construct a ThreadPoolExecutor, and one of them silently overwrites the other. The discarded executor leaks until shutdown (and its atexit shutdown still fires against a dangling reference). Compare with AgentRunManager in backend/agents/agent_run_manager.py:13-21 which correctly uses a class-level lock.

2. max_workers is silently ignored after first construction. Anywhere in the codebase, GlobalThreadPool(max_workers=20) returns the previously-constructed instance with the old size. Today line 19 creates the singleton with max_workers=5, but the default argument is max_workers=10 — so even reading the file gives no obvious answer to "what is the pool size?". Any caller passing a different value will be confused when it has no effect.

Suggested fix

import threading

class GlobalThreadPool:
    _instance = None
    _lock = threading.Lock()

    def __new__(cls, max_workers=10):
        if cls._instance is None:
            with cls._lock:
                if cls._instance is None:
                    instance = super().__new__(cls)
                    instance.pool = ThreadPoolExecutor(max_workers=max_workers)
                    atexit.register(instance.pool.shutdown)
                    cls._instance = instance
        elif max_workers != cls._instance.pool._max_workers:
            logger.warning(
                "GlobalThreadPool already initialised with max_workers=%d; "
                "requested %d will be ignored",
                cls._instance.pool._max_workers, max_workers,
            )
        return cls._instance

Or — cleaner — drop the singleton-via-__new__ pattern in favour of a module-level instance constructed once at import time.

Severity: Medium (resource leak + footgun, not a correctness bug for the current single import site).