fix: remove shared LOCK contention in monitor to prevent pod deadlock (#1754)

The monitor's update_timeline(), get_health_summary(), and
get_browser_list() all acquired the crawler pool's global LOCK to read
pool stats. That same lock is held during slow browser start/close
operations (get_crawler, janitor, close_all), causing the monitor to
block indefinitely and the pod to become unresponsive after sustained
crawling.

Replaced all three lock acquisitions in monitor.py with a lock-free
get_pool_snapshot() in crawler_pool.py that returns shallow dict copies.
Under CPython's GIL, dict.copy() and len() are atomic — safe for
read-only monitoring with at most slightly stale counts.

Author: ntohidi

deploy/docker/crawler_pool.py

@@ -22,6 +22,27 @@
 BASE_IDLE_TTL = CONFIG.get("crawler", {}).get("pool", {}).get("idle_ttl_sec", 300)
 DEFAULT_CONFIG_SIG = None  # Cached sig for default config
 
+
+def get_pool_snapshot() -> dict:
+    """Return a point-in-time snapshot of pool state for monitoring.
+
+    This is intentionally lock-free. Under CPython's GIL, reading
+    ``len(dict)``, ``dict.copy()``, and ``x is not None`` are atomic
+    operations, so the monitor can safely call this without contending
+    on the pool LOCK that is held during slow browser start/close ops.
+    The worst case is a slightly stale count, which is acceptable for
+    dashboard display purposes.
+    """
+    return {
+        "permanent": PERMANENT,
+        "permanent_sig": DEFAULT_CONFIG_SIG,
+        "hot_pool": HOT_POOL.copy(),
+        "cold_pool": COLD_POOL.copy(),
+        "last_used": LAST_USED.copy(),
+        "usage_count": USAGE_COUNT.copy(),
+    }
+
+
 def _sig(cfg: BrowserConfig) -> str:
     """Generate config signature."""
     payload = json.dumps(cfg.to_dict(), sort_keys=True, separators=(",",":"))

deploy/docker/monitor.py

@@ -149,14 +149,15 @@ async def update_timeline(self):
         recent_reqs = sum(1 for req in self.completed_requests
                          if now - req.get("end_time", 0) < 5)
 
-        # Browser counts (acquire lock to prevent race conditions)
-        from crawler_pool import PERMANENT, HOT_POOL, COLD_POOL, LOCK
-        async with LOCK:
-            browser_count = {
-                "permanent": 1 if PERMANENT else 0,
-                "hot": len(HOT_POOL),
-                "cold": len(COLD_POOL)
-            }
+        # Browser counts — lock-free snapshot to avoid contending on
+        # the pool LOCK held during slow browser start/close (issue #1754)
+        from crawler_pool import get_pool_snapshot
+        snap = get_pool_snapshot()
+        browser_count = {
+            "permanent": 1 if snap["permanent"] else 0,
+            "hot": len(snap["hot_pool"]),
+            "cold": len(snap["cold_pool"]),
+        }
 
         self.memory_timeline.append({"time": now, "value": mem_pct})
         self.requests_timeline.append({"time": now, "value": recent_reqs})
@@ -232,17 +233,17 @@ async def get_health_summary(self) -> Dict:
         # Network I/O (delta since last call)
         net = psutil.net_io_counters()
 
-        # Pool status (acquire lock to prevent race conditions)
-        from crawler_pool import PERMANENT, HOT_POOL, COLD_POOL, LOCK
-        async with LOCK:
-            # TODO: Track actual browser process memory instead of estimates
-            # These are conservative estimates based on typical Chromium usage
-            permanent_mem = 270 if PERMANENT else 0  # Estimate: ~270MB for permanent browser
-            hot_mem = len(HOT_POOL) * 180  # Estimate: ~180MB per hot pool browser
-            cold_mem = len(COLD_POOL) * 180  # Estimate: ~180MB per cold pool browser
-            permanent_active = PERMANENT is not None
-            hot_count = len(HOT_POOL)
-            cold_count = len(COLD_POOL)
+        # Pool status — lock-free snapshot (issue #1754)
+        from crawler_pool import get_pool_snapshot
+        snap = get_pool_snapshot()
+        # TODO: Track actual browser process memory instead of estimates
+        # These are conservative estimates based on typical Chromium usage
+        permanent_mem = 270 if snap["permanent"] else 0  # Estimate: ~270MB for permanent browser
+        hot_mem = len(snap["hot_pool"]) * 180  # Estimate: ~180MB per hot pool browser
+        cold_mem = len(snap["cold_pool"]) * 180  # Estimate: ~180MB per cold pool browser
+        permanent_active = snap["permanent"] is not None
+        hot_count = len(snap["hot_pool"])
+        cold_count = len(snap["cold_pool"])
 
         return {
             "container": {
@@ -287,45 +288,52 @@ def get_completed_requests(self, limit: int = 50, filter_status: str = "all") ->
 
     async def get_browser_list(self) -> List[Dict]:
         """Get detailed browser pool information."""
-        from crawler_pool import PERMANENT, HOT_POOL, COLD_POOL, LAST_USED, USAGE_COUNT, DEFAULT_CONFIG_SIG, LOCK
+        from crawler_pool import get_pool_snapshot
 
         browsers = []
         now = time.time()
 
-        # Acquire lock to prevent race conditions during iteration
-        async with LOCK:
-            if PERMANENT:
-                browsers.append({
-                    "type": "permanent",
-                    "sig": DEFAULT_CONFIG_SIG[:8] if DEFAULT_CONFIG_SIG else "unknown",
-                    "age_seconds": int(now - self.start_time),
-                    "last_used_seconds": int(now - LAST_USED.get(DEFAULT_CONFIG_SIG, now)),
-                    "memory_mb": 270,
-                    "hits": USAGE_COUNT.get(DEFAULT_CONFIG_SIG, 0),
-                    "killable": False
-                })
-
-            for sig, crawler in HOT_POOL.items():
-                browsers.append({
-                    "type": "hot",
-                    "sig": sig[:8],
-                    "age_seconds": int(now - self.start_time),  # Approximation
-                    "last_used_seconds": int(now - LAST_USED.get(sig, now)),
-                    "memory_mb": 180,  # Estimate
-                    "hits": USAGE_COUNT.get(sig, 0),
-                    "killable": True
-                })
-
-            for sig, crawler in COLD_POOL.items():
-                browsers.append({
-                    "type": "cold",
-                    "sig": sig[:8],
-                    "age_seconds": int(now - self.start_time),
-                    "last_used_seconds": int(now - LAST_USED.get(sig, now)),
-                    "memory_mb": 180,
-                    "hits": USAGE_COUNT.get(sig, 0),
-                    "killable": True
-                })
+        # Lock-free snapshot — iterates over copies (issue #1754)
+        snap = get_pool_snapshot()
+        permanent = snap["permanent"]
+        permanent_sig = snap["permanent_sig"]
+        hot_pool = snap["hot_pool"]
+        cold_pool = snap["cold_pool"]
+        last_used = snap["last_used"]
+        usage_count = snap["usage_count"]
+
+        if permanent:
+            browsers.append({
+                "type": "permanent",
+                "sig": permanent_sig[:8] if permanent_sig else "unknown",
+                "age_seconds": int(now - self.start_time),
+                "last_used_seconds": int(now - last_used.get(permanent_sig, now)),
+                "memory_mb": 270,
+                "hits": usage_count.get(permanent_sig, 0),
+                "killable": False
+            })
+
+        for sig, crawler in hot_pool.items():
+            browsers.append({
+                "type": "hot",
+                "sig": sig[:8],
+                "age_seconds": int(now - self.start_time),  # Approximation
+                "last_used_seconds": int(now - last_used.get(sig, now)),
+                "memory_mb": 180,  # Estimate
+                "hits": usage_count.get(sig, 0),
+                "killable": True
+            })
+
+        for sig, crawler in cold_pool.items():
+            browsers.append({
+                "type": "cold",
+                "sig": sig[:8],
+                "age_seconds": int(now - self.start_time),
+                "last_used_seconds": int(now - last_used.get(sig, now)),
+                "memory_mb": 180,
+                "hits": usage_count.get(sig, 0),
+                "killable": True
+            })
 
         return browsers