Merge pull request #1728 from codeflash-ai/fix/java/sqlite-locked-error

misrasaurabh1 · web-flow · commit cc2e5fbe4e6e · 2026-03-02T17:59:57.000-08:00
fix(java): resolve SQLite 'database is locked' errors across the pipeline
diff --git a/codeflash/languages/java/test_runner.py b/codeflash/languages/java/test_runner.py
@@ -11,6 +11,7 @@
 import os
 import re
 import shutil
+import signal
 import subprocess
 import tempfile
 import uuid
@@ -46,6 +47,83 @@
 _VALID_JAVA_CLASS_NAME = re.compile(r"^[a-zA-Z_$][a-zA-Z0-9_$.]*$")
 
 
+def _run_cmd_kill_pg_on_timeout(
+    cmd: list[str],
+    *,
+    cwd: Path | None = None,
+    env: dict[str, str] | None = None,
+    timeout: int | None = None,
+    text: bool = True,
+) -> subprocess.CompletedProcess:
+    """Run a command, killing its entire process group on timeout.
+
+    Unlike subprocess.run(), this function uses start_new_session=True so the
+    child process gets its own process group.  When the timeout fires we send
+    SIGTERM (then SIGKILL) to the whole process group, not just the process
+    itself.  This is critical for Maven, which forks child JVM processes
+    (Maven Surefire forks) that would otherwise become orphaned when the Maven
+    parent is killed by a plain subprocess.run() timeout.  Orphaned JVMs keep
+    SQLite file-handles open, causing "database is locked" errors.
+
+    Args:
+        cmd: Command and arguments.
+        cwd: Working directory.
+        env: Environment variables.
+        timeout: Seconds to wait before killing the process group.
+        text: If True, decode stdout/stderr as text.
+
+    Returns:
+        CompletedProcess.  On timeout, returncode is -2 and stderr contains a
+        human-readable explanation.
+
+    """
+    proc = subprocess.Popen(
+        cmd,
+        cwd=cwd,
+        env=env,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        text=text,
+        start_new_session=True,  # puts proc in its own process group
+    )
+    try:
+        stdout, stderr = proc.communicate(timeout=timeout)
+        return subprocess.CompletedProcess(args=cmd, returncode=proc.returncode, stdout=stdout, stderr=stderr)
+    except subprocess.TimeoutExpired:
+        # Kill the entire process group so Maven's forked Surefire JVMs don't
+        # become orphans that keep the SQLite database locked.
+        pgid = None
+        try:
+            pgid = os.getpgid(proc.pid)
+            os.killpg(pgid, signal.SIGTERM)
+        except (ProcessLookupError, OSError):
+            proc.kill()
+        # Give processes a few seconds to shut down gracefully before SIGKILL.
+        try:
+            proc.wait(timeout=5)
+        except subprocess.TimeoutExpired:
+            if pgid is not None:
+                try:
+                    os.killpg(pgid, signal.SIGKILL)
+                except (ProcessLookupError, OSError):
+                    pass
+            else:
+                proc.kill()
+            proc.wait()
+        # Drain pipes so we don't leave zombie pipe buffers.
+        try:
+            stdout_data = proc.stdout.read() if proc.stdout else ""
+            stderr_data = proc.stderr.read() if proc.stderr else ""
+        except Exception:
+            stdout_data, stderr_data = "", ""
+        return subprocess.CompletedProcess(
+            args=cmd,
+            returncode=-2,
+            stdout=stdout_data,
+            stderr=f"Process group killed after timeout ({timeout}s): {stderr_data}",
+        )
+
+
 def _validate_java_class_name(class_name: str) -> bool:
     """Validate that a string is a valid Java class name.
 
@@ -505,14 +583,7 @@ def _compile_tests(
     logger.debug("Compiling tests: %s in %s", " ".join(cmd), project_root)
 
     try:
-        return subprocess.run(
-            cmd, check=False, cwd=project_root, env=env, capture_output=True, text=True, timeout=timeout
-        )
-    except subprocess.TimeoutExpired:
-        logger.exception("Maven compilation timed out after %d seconds", timeout)
-        return subprocess.CompletedProcess(
-            args=cmd, returncode=-2, stdout="", stderr=f"Compilation timed out after {timeout} seconds"
-        )
+        return _run_cmd_kill_pg_on_timeout(cmd, cwd=project_root, env=env, timeout=timeout)
     except Exception as e:
         logger.exception("Maven compilation failed: %s", e)
         return subprocess.CompletedProcess(args=cmd, returncode=-1, stdout="", stderr=str(e))
@@ -548,9 +619,7 @@ def _get_test_classpath(
     logger.debug("Getting classpath: %s", " ".join(cmd))
 
     try:
-        result = subprocess.run(
-            cmd, check=False, cwd=project_root, env=env, capture_output=True, text=True, timeout=timeout
-        )
+        result = _run_cmd_kill_pg_on_timeout(cmd, cwd=project_root, env=env, timeout=timeout)
 
         if result.returncode != 0:
             logger.error("Failed to get classpath: %s", result.stderr)
@@ -600,9 +669,6 @@ def _get_test_classpath(
 
         return os.pathsep.join(cp_parts)
 
-    except subprocess.TimeoutExpired:
-        logger.exception("Getting classpath timed out")
-        return None
     except Exception as e:
         logger.exception("Failed to get classpath: %s", e)
         return None
@@ -804,14 +870,7 @@ def _run_tests_direct(
         logger.debug("Running tests directly: java -cp ... ConsoleLauncher --select-class %s", test_classes)
 
     try:
-        return subprocess.run(
-            cmd, check=False, cwd=working_dir, env=env, capture_output=True, text=True, timeout=timeout
-        )
-    except subprocess.TimeoutExpired:
-        logger.exception("Direct test execution timed out after %d seconds", timeout)
-        return subprocess.CompletedProcess(
-            args=cmd, returncode=-2, stdout="", stderr=f"Test execution timed out after {timeout} seconds"
-        )
+        return _run_cmd_kill_pg_on_timeout(cmd, cwd=working_dir, env=env, timeout=timeout)
     except Exception as e:
         logger.exception("Direct test execution failed: %s", e)
         return subprocess.CompletedProcess(args=cmd, returncode=-1, stdout="", stderr=str(e))
@@ -1511,9 +1570,13 @@ def _run_maven_tests(
     logger.debug("Running Maven command: %s in %s", " ".join(cmd), project_root)
 
     try:
-        result = subprocess.run(
-            cmd, check=False, cwd=project_root, env=env, capture_output=True, text=True, timeout=timeout
-        )
+        # Use _run_cmd_kill_pg_on_timeout instead of subprocess.run so that on
+        # timeout we kill the entire Maven process GROUP (including forked Surefire
+        # JVMs).  With plain subprocess.run(), only the Maven parent is killed and
+        # the child JVMs become orphaned, holding the SQLite result file open and
+        # causing "database is locked" errors when Python reads the file immediately
+        # after Maven is killed.
+        result = _run_cmd_kill_pg_on_timeout(cmd, cwd=project_root, env=env, timeout=timeout)
 
         # Check if Maven failed due to compilation errors (not just test failures)
         if result.returncode != 0:
@@ -1546,11 +1609,6 @@ def _run_maven_tests(
 
         return result
 
-    except subprocess.TimeoutExpired:
-        logger.exception("Maven test execution timed out after %d seconds", timeout)
-        return subprocess.CompletedProcess(
-            args=cmd, returncode=-2, stdout="", stderr=f"Test execution timed out after {timeout} seconds"
-        )
     except Exception as e:
         logger.exception("Maven test execution failed: %s", e)
         return subprocess.CompletedProcess(args=cmd, returncode=-1, stdout="", stderr=str(e))
diff --git a/tests/test_languages/test_java/test_run_and_parse.py b/tests/test_languages/test_java/test_run_and_parse.py
@@ -7,7 +7,10 @@
 """
 
 import os
+import signal
 import sqlite3
+import sys
+import time
 from argparse import Namespace
 from pathlib import Path
 
@@ -653,3 +656,95 @@ def test_performance_multiple_test_methods_inner_loop(self, java_project):
             f"total_passed_runtime {total_runtime / 1_000_000:.3f}ms not close to expected "
             f"{expected_total_ns / 1_000_000:.1f}ms (2 methods × min of 4 runtimes × 10ms, ±3%)"
         )
+
+
+# ---------------------------------------------------------------------------
+# Tests for _run_cmd_kill_pg_on_timeout
+# Root-cause fix for "database is locked": when Maven times out, we must kill
+# its forked Surefire child JVMs (entire process group), not just the Maven
+# parent process.  Plain subprocess.run() only kills the parent, orphaning the
+# child JVMs which continue to hold the SQLite file open.
+# ---------------------------------------------------------------------------
+
+class TestRunCmdKillPgOnTimeout:
+    """Unit tests for _run_cmd_kill_pg_on_timeout in test_runner.py."""
+
+    def _helper(self):
+        from codeflash.languages.java.test_runner import _run_cmd_kill_pg_on_timeout
+        return _run_cmd_kill_pg_on_timeout
+
+    def test_successful_command_returns_correct_output(self):
+        """A command that finishes normally should return its stdout/stderr/returncode."""
+        fn = self._helper()
+        result = fn(["echo", "hello"], timeout=10)
+        assert result.returncode == 0
+        assert "hello" in result.stdout
+
+    def test_failing_command_returns_nonzero_returncode(self):
+        """A command that exits non-zero should propagate the returncode."""
+        fn = self._helper()
+        result = fn(["false"], timeout=10)
+        assert result.returncode != 0
+
+    @pytest.mark.skipif(sys.platform == "win32", reason="Process groups are POSIX only")
+    def test_timeout_kills_child_processes_not_just_parent(self, tmp_path):
+        """On timeout, child processes must be killed (not left orphaned).
+
+        We start a shell script that forks a long-running child (sleep 60).
+        The script writes the child PID to a file.  We set a short timeout so
+        _run_cmd_kill_pg_on_timeout kills the entire process group.  After the
+        call returns, the child must be dead (not still running).
+        """
+        fn = self._helper()
+        child_pid_file = tmp_path / "child.pid"
+        # Shell: start sleep in background, write its PID, then sleep ourselves
+        # so the parent also runs long enough to be timed out.
+        script = (
+            f"sleep 60 & echo $! > {child_pid_file}; sleep 60"
+        )
+        result = fn(["bash", "-c", script], timeout=2)
+
+        # The result should indicate a timeout (returncode -2)
+        assert result.returncode == -2
+
+        # Wait briefly for the child PID file to be written (race between
+        # the script writing it and us being killed).
+        deadline = time.monotonic() + 3
+        while not child_pid_file.exists() and time.monotonic() < deadline:
+            time.sleep(0.1)
+
+        if not child_pid_file.exists():
+            # Script was killed before it could write the PID — that's fine;
+            # it means the child never started, so no orphan problem.
+            return
+
+        child_pid_str = child_pid_file.read_text().strip()
+        if not child_pid_str.isdigit():
+            return
+
+        child_pid = int(child_pid_str)
+        # Give the child a moment to be killed by the process-group SIGKILL.
+        time.sleep(0.5)
+
+        # Check if the child is still alive by sending signal 0.
+        try:
+            os.kill(child_pid, 0)
+            # If we get here the process is still running — that's the bug we fixed.
+            assert False, (
+                f"Child process {child_pid} is still running after timeout kill; "
+                "orphaned JVMs would keep the SQLite file locked"
+            )
+        except ProcessLookupError:
+            pass  # Process is dead — expected behaviour
+
+    def test_returncode_is_minus_two_on_timeout(self):
+        """Timed-out commands must return returncode=-2 (our convention)."""
+        fn = self._helper()
+        result = fn(["sleep", "60"], timeout=1)
+        assert result.returncode == -2
+
+    def test_timeout_message_in_stderr(self):
+        """The stderr of a timed-out command should describe the timeout."""
+        fn = self._helper()
+        result = fn(["sleep", "60"], timeout=1)
+        assert "timeout" in result.stderr.lower() or "killed" in result.stderr.lower()
