fix: interrupt PostgreSQL cache subprocesses

Author: renecannao

scripts/pg_compat/fetch_libpg_query.sh

@@ -13,6 +13,7 @@ LOCK_RECLAIM_DIR=
 LOCK_RECLAIM_FILE=
 LOCK_HELD=0
 FILE_CONTENT=
+ACTIVE_CHILD_PID=
 
 usage() {
     echo "Usage: $0 [--with-postgres-source]" >&2
@@ -166,10 +167,51 @@ acquire_lock() {
     exit 1
 }
 
+handle_signal() {
+    local signal_name="$1"
+    local exit_status="$2"
+    local child_pid="$ACTIVE_CHILD_PID"
+
+    trap - HUP INT TERM
+    ACTIVE_CHILD_PID=
+    if [[ -n "$child_pid" ]]; then
+        if ! kill "-${signal_name}" -- "-${child_pid}" 2>/dev/null; then
+            kill "-${signal_name}" "$child_pid" 2>/dev/null || true
+        fi
+        wait "$child_pid" 2>/dev/null || true
+    fi
+    exit "$exit_status"
+}
+
+run_interruptible() {
+    local child_pid
+    local child_status
+
+    python3 -c '
+import os
+import signal
+import sys
+
+os.setsid()
+for signal_number in (signal.SIGHUP, signal.SIGINT, signal.SIGTERM):
+    signal.signal(signal_number, signal.SIG_DFL)
+os.execvp(sys.argv[1], sys.argv[1:])
+' "$@" &
+    child_pid=$!
+    ACTIVE_CHILD_PID="$child_pid"
+    if wait "$child_pid"; then
+        child_status=0
+    else
+        child_status=$?
+    fi
+    ACTIVE_CHILD_PID=
+    return "$child_status"
+}
+
 trap release_lock EXIT
-trap 'exit 129' HUP
-trap 'exit 130' INT
-trap 'exit 143' TERM
+trap 'handle_signal HUP 129' HUP
+trap 'handle_signal INT 130' INT
+trap 'handle_signal TERM 143' TERM
 
 LIBPG_QUERY_URL="$(
     python3 - "$PINS" <<'PY'
@@ -248,13 +290,13 @@ fetch_libpg_query() {
             echo "Cache path exists but is not a Git checkout: ${checkout}" >&2
             exit 1
         fi
-        git clone --no-checkout "$LIBPG_QUERY_URL" "$checkout"
+        run_interruptible git clone --no-checkout "$LIBPG_QUERY_URL" "$checkout"
     fi
 
-    git -C "$checkout" remote set-url origin "$LIBPG_QUERY_URL"
+    run_interruptible git -C "$checkout" remote set-url origin "$LIBPG_QUERY_URL"
     echo "Fetching libpg_query ${role}: ${branch} at ${commit}"
-    git -C "$checkout" fetch --force --no-tags origin "$commit"
-    git -C "$checkout" checkout --detach --force "$commit"
+    run_interruptible git -C "$checkout" fetch --force --no-tags origin "$commit"
+    run_interruptible git -C "$checkout" checkout --detach --force "$commit"
 
     actual_head="$(git -C "$checkout" rev-parse HEAD)"
     if [[ "$actual_head" != "$commit" ]]; then
@@ -301,7 +343,7 @@ fetch_postgres_source() {
 
     if [[ ! -f "$archive" ]]; then
         download_tmp="$(mktemp "${postgres_root}/.postgresql-${pg_version}.download.XXXXXX")"
-        if ! curl --fail --location --retry 3 --output "$download_tmp" "$archive_url"; then
+        if ! run_interruptible curl --fail --location --retry 3 --output "$download_tmp" "$archive_url"; then
             rm -f "$download_tmp"
             return 1
         fi
@@ -316,7 +358,7 @@ fetch_postgres_source() {
 
     if [[ ! -d "$source_dir" ]]; then
         extract_tmp="$(mktemp -d "${postgres_root}/.postgresql-${pg_version}.extract.XXXXXX")"
-        if ! tar -xjf "$archive" -C "$extract_tmp"; then
+        if ! run_interruptible tar -xjf "$archive" -C "$extract_tmp"; then
             rm -rf "$extract_tmp"
             return 1
         fi

tests/pg_compat/test_common.py

@@ -236,7 +236,15 @@ def create_harness(self, directory, pins):
         printf 'PG_VERSION = 18.4\\nPG_VERSION_NUM = 180004\\n' > "$checkout/Makefile"
     fi
     if [[ "${STUB_BLOCK_STARTED:-}" != "" && "$(basename "$checkout")" == "previous" ]]; then
+        if [[ "${STUB_BLOCK_PID:-}" != "" ]]; then
+            printf '%s\n' "$$" > "$STUB_BLOCK_PID"
+        fi
         touch "$STUB_BLOCK_STARTED"
+        if [[ "${STUB_BLOCK_FOREVER:-}" != "" ]]; then
+            while true; do
+                sleep 60
+            done
+        fi
         while [[ ! -e "$STUB_BLOCK_RELEASE" ]]; do
             sleep 0.02
         done
@@ -335,6 +343,19 @@ def create_lock(self, cache, owner=None):
             (lock_dir / "owner").write_text(owner, encoding="utf-8")
         return lock_dir
 
+    def kill_process_safely(self, pid):
+        try:
+            process_group = os.getpgid(pid)
+        except ProcessLookupError:
+            return
+        try:
+            if process_group == pid:
+                os.killpg(process_group, signal.SIGKILL)
+            else:
+                os.kill(pid, signal.SIGKILL)
+        except ProcessLookupError:
+            pass
+
     def test_bad_download_is_not_published_and_poisoned_cache_recovers(self):
         good_content = b"good archive"
         expected_hash = hashlib.sha256(good_content).hexdigest()
@@ -484,9 +505,10 @@ def test_terminated_fetch_releases_lock_without_continuing(self):
                 directory, valid_pins()
             )
             started = root / "started"
-            release = root / "release"
+            child_pid_path = root / "child-pid"
             environment["STUB_BLOCK_STARTED"] = str(started)
-            environment["STUB_BLOCK_RELEASE"] = str(release)
+            environment["STUB_BLOCK_PID"] = str(child_pid_path)
+            environment["STUB_BLOCK_FOREVER"] = "1"
 
             process = subprocess.Popen(
                 [str(FETCH_SCRIPT)],
@@ -495,24 +517,42 @@ def test_terminated_fetch_releases_lock_without_continuing(self):
                 text=True,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,
+                start_new_session=True,
             )
-            deadline = time.monotonic() + 5
-            while not started.exists() and time.monotonic() < deadline:
-                time.sleep(0.02)
-            self.assertTrue(started.exists(), "fetch did not reach locked operation")
-
-            os.kill(process.pid, signal.SIGTERM)
-            release.touch()
-            process.communicate(timeout=5)
-
-            self.assertNotEqual(process.returncode, 0)
-            self.assertFalse((cache / ".pg_compat.lock").exists())
-
-            retry_environment = environment.copy()
-            retry_environment.pop("STUB_BLOCK_STARTED")
-            retry_environment.pop("STUB_BLOCK_RELEASE")
-            retry = self.run_fetch(retry_environment)
-            self.assertEqual(retry.returncode, 0, retry.stderr)
+            child_pid = None
+            try:
+                deadline = time.monotonic() + 5
+                while (
+                    (not started.exists() or not child_pid_path.exists())
+                    and time.monotonic() < deadline
+                ):
+                    time.sleep(0.02)
+                self.assertTrue(
+                    started.exists(), "fetch did not reach locked operation"
+                )
+                self.assertTrue(
+                    child_pid_path.exists(), "blocked child did not record its PID"
+                )
+                child_pid = int(
+                    child_pid_path.read_text(encoding="utf-8").strip()
+                )
+
+                os.kill(process.pid, signal.SIGTERM)
+                try:
+                    process.communicate(timeout=2)
+                except subprocess.TimeoutExpired:
+                    self.fail("terminated fetch did not exit promptly")
+
+                self.assertEqual(process.returncode, 143)
+                with self.assertRaises(ProcessLookupError):
+                    os.kill(child_pid, 0)
+                self.assertFalse((cache / ".pg_compat.lock").exists())
+            finally:
+                if process.poll() is None:
+                    self.kill_process_safely(process.pid)
+                    process.communicate(timeout=5)
+                if child_pid is not None:
+                    self.kill_process_safely(child_pid)
 
 
 if __name__ == "__main__":