Merge d11ec38acb into e63fc85b7b

litellm/proxy/db/prisma_client.py

@@ -412,7 +412,8 @@ class PrismaWrapper:
         stuck on TCP close.
 
         Sends SIGTERM for graceful shutdown, waits briefly, then SIGKILL as
-        a backstop.
+        a backstop, then reaps the PID so it does not linger as a zombie
+        (#33414, #14739, #10216).
         """
         if pid <= 0:
             return
@@ -434,6 +435,47 @@ class PrismaWrapper:
             )
         except (ProcessLookupError, PermissionError, OSError):
             pass  # Exited after SIGTERM — expected
+        await PrismaWrapper._reap_engine_process(pid)
+
+    @staticmethod
+    async def _reap_engine_process(pid: int) -> None:
+        """Reap the killed engine so it doesn't linger as a zombie.
+
+        A signalled child stays in the process table as ``<defunct>`` until its
+        parent wait()s on it. We retire the engine by signalling its PID rather
+        than calling prisma-client-py's ``disconnect()``, so nothing else ever
+        wait()s on that Popen — leaving exactly one zombie per reconnect, which
+        accumulates for the life of the container (#33414, #14739, #10216).
+
+        This does NOT reintroduce the blocking ``disconnect()`` problem this
+        class exists to avoid: that call blocks for 30-120s because it waits on
+        a *live* engine stuck on TCP close. By this point the engine has already
+        been SIGKILLed, so it is dead and WNOHANG collects it on the first poll.
+        The bounded retry only covers the brief window between the signal being
+        delivered and the kernel finishing the exit.
+        """
+        wnohang: Final[int | None] = getattr(os, "WNOHANG", None)
+        if wnohang is None:
+            return  # Windows: no POSIX zombies, nothing to reap.
+        deadline: Final = time.monotonic() + 2.0
+        while True:
+            try:
+                reaped, _status = os.waitpid(pid, wnohang)
+            except ChildProcessError:
+                return  # Not our child, or already reaped — nothing to do.
+            except OSError:
+                return
+            if reaped == pid:
+                return  # Collected.
+            # reaped == 0: still exiting. Yield rather than spin.
+            if time.monotonic() >= deadline:
+                verbose_proxy_logger.warning(
+                    "prisma-query-engine PID %s did not exit after SIGKILL; "
+                    "leaving it unreaped rather than blocking the event loop.",
+                    pid,
+                )
+                return
+            await asyncio.sleep(0.05)
 
     def _extract_token_from_db_url(self, db_url: str | None) -> str | None:
         """

tests/unit/proxy/db/test_prisma_engine_kill_reap.py

@@ -0,0 +1,94 @@
+"""Engine-reap coverage for PrismaWrapper._kill_engine_process (#33414).
+
+These tests drive real child processes so they assert what the reconnect path
+actually leaves behind in the process table, not which syscalls it made.
+"""
+
+import os
+import subprocess
+import sys
+from collections.abc import Iterator
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from litellm.proxy.db.prisma_client import PrismaWrapper
+
+pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="POSIX zombies only exist on POSIX")
+
+
+@pytest.fixture(autouse=True)
+def mock_prisma_binary():
+    mock_module = MagicMock()
+    with patch.dict(sys.modules, {"prisma": mock_module}):
+        yield mock_module
+
+
+@pytest.fixture
+def engine_stand_in() -> Iterator[subprocess.Popen[bytes]]:
+    """A long-lived child standing in for prisma-query-engine."""
+    proc = subprocess.Popen(["sleep", "60"])
+    try:
+        yield proc
+    finally:
+        if proc.poll() is None:
+            proc.kill()
+            proc.wait()
+
+
+def _is_unreaped_child(pid: int) -> bool:
+    """True while `pid` is still our child in the process table (alive or zombie)."""
+    try:
+        os.kill(pid, 0)
+    except ProcessLookupError:
+        return False
+    return True
+
+
+@pytest.mark.asyncio
+async def test_kill_engine_process_leaves_no_zombie(engine_stand_in):
+    """Regression for #33414/#14739/#10216: the killed engine must not linger as <defunct>."""
+    pid = engine_stand_in.pid
+
+    await PrismaWrapper._kill_engine_process(pid)
+
+    assert not _is_unreaped_child(pid)
+    with pytest.raises(ChildProcessError):
+        os.waitpid(pid, os.WNOHANG)
+
+
+@pytest.mark.asyncio
+async def test_reap_tolerates_already_reaped_child(engine_stand_in):
+    """ChildProcessError means someone else already collected it, which is not an error."""
+    engine_stand_in.kill()
+    engine_stand_in.wait()
+
+    assert await PrismaWrapper._reap_engine_process(engine_stand_in.pid) is None
+
+
+@pytest.mark.asyncio
+async def test_reap_gives_up_on_a_live_child_instead_of_blocking(engine_stand_in):
+    """A child that never exits is left alone after the bounded wait, not waited on forever."""
+    await PrismaWrapper._reap_engine_process(engine_stand_in.pid)
+
+    assert engine_stand_in.poll() is None, "a live engine must not be killed by the reap"
+
+
+@pytest.mark.asyncio
+async def test_reap_is_a_no_op_without_wnohang(engine_stand_in):
+    """Windows has no os.WNOHANG, so the reap must leave the process table untouched."""
+    import litellm.proxy.db.prisma_client as pc
+
+    engine_stand_in.kill()
+    with patch.object(pc.os, "WNOHANG", None, create=False):
+        await PrismaWrapper._reap_engine_process(engine_stand_in.pid)
+
+    assert _is_unreaped_child(engine_stand_in.pid)
+    assert engine_stand_in.wait() != 0
+
+
+@pytest.mark.asyncio
+async def test_reap_swallows_unexpected_oserror_from_waitpid():
+    """A non-ECHILD OSError from waitpid must not abort reconnect."""
+    with patch("os.waitpid", side_effect=OSError("EINVAL")):
+        assert await PrismaWrapper._reap_engine_process(4242) is None