diff --git a/gateway/run.py b/gateway/run.py index 367adbe61db19..0da6e66b0eb7a 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -709,6 +709,41 @@ def _ensure_ssl_certs() -> None: os.environ["SSL_CERT_FILE"] = candidate return +def _raise_fd_soft_limit(min_soft: int = 4096) -> None: + """Raise RLIMIT_NOFILE soft limit toward the hard limit (Unix only). + + macOS ships a default soft limit of 256, which is easily exhausted by + multiple MCP subprocesses + per-profile gateways (#30230). Bumping + early prevents EMFILE crashes in session save / kanban dispatch and + complements the per-shutdown auxiliary-client reap added in #14210. + + Bumps to ``min(min_soft, hard)``; if the soft limit is already above + ``min_soft``, leaves it alone. All failures are swallowed silently: + Windows has no ``resource`` module, sandboxed environments may + forbid ``setrlimit``, and a missed raise is a soft regression (the + original symptom — EMFILE under load — is what's getting fixed). + """ + try: + import resource + except ImportError: + return # Windows / no POSIX resource module + try: + soft, hard = resource.getrlimit(resource.RLIMIT_NOFILE) + except (OSError, ValueError): + return + if soft >= min_soft: + return + target = min_soft if hard == resource.RLIM_INFINITY else min(min_soft, hard) + if target <= soft: + return + try: + resource.setrlimit(resource.RLIMIT_NOFILE, (target, hard)) + except (OSError, ValueError): + # Sandboxed envs / hard-limit kernels reject the bump; the + # original EMFILE symptom is still what surfaces if so. + pass + + def _home_target_env_var(platform_name: str) -> str: """Return the configured home-target env var for a platform. @@ -739,6 +774,7 @@ def _restart_notification_pending() -> bool: os.environ["_HERMES_GATEWAY"] = "1" _ensure_ssl_certs() +_raise_fd_soft_limit() # Add parent directory to path sys.path.insert(0, str(Path(__file__).parent.parent)) diff --git a/tests/gateway/test_fd_soft_limit.py b/tests/gateway/test_fd_soft_limit.py new file mode 100644 index 0000000000000..344d63d3ce104 --- /dev/null +++ b/tests/gateway/test_fd_soft_limit.py @@ -0,0 +1,124 @@ +"""Tests for RLIMIT_NOFILE soft-limit bump in gateway/run.py (#30230).""" + +from __future__ import annotations + +import textwrap +from types import ModuleType +from unittest.mock import patch + + +def _load_raise_fd_soft_limit(): + """Replicate the helper in an isolated module. + + gateway/run.py has heavy imports; tests/gateway/test_ssl_certs.py uses + the same pattern. The body below must stay in sync with the production + function in gateway/run.py. + """ + code = textwrap.dedent("""\ + def _raise_fd_soft_limit(min_soft=4096): + try: + import resource + except ImportError: + return + try: + soft, hard = resource.getrlimit(resource.RLIMIT_NOFILE) + except (OSError, ValueError): + return + if soft >= min_soft: + return + target = min_soft if hard == resource.RLIM_INFINITY else min(min_soft, hard) + if target <= soft: + return + try: + resource.setrlimit(resource.RLIMIT_NOFILE, (target, hard)) + except (OSError, ValueError): + pass + """) + mod = ModuleType("_fd_helper") + exec(code, mod.__dict__) + return mod._raise_fd_soft_limit + + +class TestRaiseFdSoftLimit: + def test_bumps_from_256_to_4096_when_hard_is_infinity(self): + fn = _load_raise_fd_soft_limit() + import resource + + calls = [] + with patch.object(resource, "getrlimit", return_value=(256, resource.RLIM_INFINITY)), \ + patch.object(resource, "setrlimit", side_effect=lambda r, v: calls.append((r, v))): + fn() + assert calls == [(resource.RLIMIT_NOFILE, (4096, resource.RLIM_INFINITY))] + + def test_caps_at_hard_when_hard_below_target(self): + fn = _load_raise_fd_soft_limit() + import resource + + calls = [] + with patch.object(resource, "getrlimit", return_value=(256, 1024)), \ + patch.object(resource, "setrlimit", side_effect=lambda r, v: calls.append((r, v))): + fn() + assert calls == [(resource.RLIMIT_NOFILE, (1024, 1024))] + + def test_noop_when_soft_already_high(self): + fn = _load_raise_fd_soft_limit() + import resource + + calls = [] + with patch.object(resource, "getrlimit", return_value=(8192, resource.RLIM_INFINITY)), \ + patch.object(resource, "setrlimit", side_effect=lambda r, v: calls.append((r, v))): + fn() + assert calls == [] + + def test_noop_when_soft_equals_hard_below_min(self): + fn = _load_raise_fd_soft_limit() + import resource + + calls = [] + with patch.object(resource, "getrlimit", return_value=(256, 256)), \ + patch.object(resource, "setrlimit", side_effect=lambda r, v: calls.append((r, v))): + fn() + # target == hard == 256, but target <= soft (also 256) so no call. + assert calls == [] + + def test_swallows_getrlimit_error(self): + fn = _load_raise_fd_soft_limit() + import resource + + calls = [] + with patch.object(resource, "getrlimit", side_effect=OSError("denied")), \ + patch.object(resource, "setrlimit", side_effect=lambda r, v: calls.append((r, v))): + fn() # must not raise + assert calls == [] + + def test_swallows_setrlimit_error(self): + fn = _load_raise_fd_soft_limit() + import resource + + with patch.object(resource, "getrlimit", return_value=(256, resource.RLIM_INFINITY)), \ + patch.object(resource, "setrlimit", side_effect=OSError("EPERM")): + fn() # must not raise + + def test_custom_min_soft_threshold(self): + fn = _load_raise_fd_soft_limit() + import resource + + calls = [] + with patch.object(resource, "getrlimit", return_value=(256, resource.RLIM_INFINITY)), \ + patch.object(resource, "setrlimit", side_effect=lambda r, v: calls.append((r, v))): + fn(min_soft=2048) + assert calls == [(resource.RLIMIT_NOFILE, (2048, resource.RLIM_INFINITY))] + + +class TestProductionBodyMatchesReplica: + """Pin the production source so the in-test replica can't silently drift.""" + + def test_production_function_body_keywords(self): + from pathlib import Path + src = Path(__file__).resolve().parents[2] / "gateway" / "run.py" + text = src.read_text() + assert "def _raise_fd_soft_limit(" in text + assert "RLIMIT_NOFILE" in text + assert "RLIM_INFINITY" in text + # Helper is wired into module init right after _ensure_ssl_certs(). + assert "_raise_fd_soft_limit()" in text diff --git a/tests/tools/test_local_interrupt_cleanup.py b/tests/tools/test_local_interrupt_cleanup.py index 67d9e9e6b54b9..b65e989286dd4 100644 --- a/tests/tools/test_local_interrupt_cleanup.py +++ b/tests/tools/test_local_interrupt_cleanup.py @@ -96,9 +96,19 @@ def fake_killpg(pgid, sig): assert killpg_calls == [(67890, signal.SIGTERM), (67890, 0)] +@pytest.mark.timeout(90) def test_wait_for_process_kills_subprocess_on_keyboardinterrupt(): """When KeyboardInterrupt arrives mid-poll, the subprocess group must be - killed before the exception is re-raised.""" + killed before the exception is re-raised. + + The test's internal timeouts sum to ~50s (5s subprocess discovery + + 15s worker-thread join + 30s process-group-exit poll), which exceeds + the suite's 30s pytest-timeout default. Under heavy xdist load the + cleanup chain (SIGTERM → reap → SIGKILL → reap) can take long enough + that the 30s cap fires before ``_wait_for_pgid_exit`` finishes its + first poll cycle. A 90s ceiling gives the test its full budget plus + generous headroom for CI scheduling jitter. + """ env = LocalEnvironment(cwd="/tmp") try: result_holder = {}