Merge pull request #994 from Yo-DDV/fix/launcher-lock-pidfd-compat

Fix launcher lock on Python without pidfd APIs

Author: ilysenko

CHANGELOG.md

@@ -35,6 +35,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Fixed
 
+- Launcher startup no longer requires Python's pidfd wrappers for normal
+  launcher lock acquire and release. Pidfd remains reserved for the
+  identity-verified stale Electron termination path.
 - Approval notifications now preserve the upstream Approve, Approve for
   session, and Decline actions on Linux. A small freedesktop notification
   bridge forwards the action and close signals that Electron's Linux

launcher/start.sh.template

@@ -3894,9 +3894,6 @@ parent_pid = int(sys.argv[1])
 lock_path, wait_text, status_path = sys.argv[2:]
 wait_seconds = int(wait_text)
 
-if not hasattr(os, "pidfd_open") or not hasattr(signal, "pidfd_send_signal"):
-    sys.exit(1)
-
 libc_name = ctypes.util.find_library("c") or "libc.so.6"
 try:
     libc = ctypes.CDLL(libc_name, use_errno=True)
@@ -3999,49 +3996,16 @@ launcher_lock_helper_is_active() {
 }
 
 stop_launcher_lock_helper() {
-    python3 - \
-        "$LAUNCHER_LOCK_HELPER_PID" \
-        "$LAUNCHER_LOCK_HELPER_START_TIME" \
-        "${BASHPID:-$$}" <<'PY'
-import os
-import select
-import signal
-import sys
-
-pid = int(sys.argv[1])
-expected_start_time = sys.argv[2]
-expected_parent = int(sys.argv[3])
+    local actual_start_time
+    local cmdline
 
-if not hasattr(os, "pidfd_open") or not hasattr(signal, "pidfd_send_signal"):
-    sys.exit(1)
-try:
-    pidfd = os.pidfd_open(pid, 0)
-    with open(f"/proc/{pid}/stat", "r", encoding="utf-8") as handle:
-        fields = handle.read().rsplit(") ", 1)[1].split()
-    if fields[19] != expected_start_time or int(fields[1]) != expected_parent:
-        os.close(pidfd)
-        sys.exit(1)
-    poller = select.poll()
-    poller.register(pidfd, select.POLLIN)
-    exited = False
-    for requested_signal, wait_ms in (
-        (signal.SIGUSR1, 500),
-        (signal.SIGTERM, 500),
-        (signal.SIGKILL, 500),
-    ):
-        try:
-            signal.pidfd_send_signal(pidfd, requested_signal)
-        except ProcessLookupError:
-            exited = True
-            break
-        if poller.poll(wait_ms):
-            exited = True
-            break
-    os.close(pidfd)
-    sys.exit(0 if exited else 1)
-except (OSError, IndexError, ValueError):
-    sys.exit(1)
-PY
+    [[ "$LAUNCHER_LOCK_HELPER_PID" =~ ^[0-9]+$ ]] || return 1
+    actual_start_time="$(pid_start_time "$LAUNCHER_LOCK_HELPER_PID" 2>/dev/null || true)"
+    [ -n "$actual_start_time" ] && [ "$actual_start_time" = "$LAUNCHER_LOCK_HELPER_START_TIME" ] || return 1
+    pid_parent_matches "$LAUNCHER_LOCK_HELPER_PID" "${BASHPID:-$$}" || return 1
+    cmdline="$(pid_cmdline "$LAUNCHER_LOCK_HELPER_PID")"
+    [[ "$cmdline" == *" $APP_STATE_DIR/launcher.lock "*" $LAUNCHER_LOCK_STATUS_PATH"* ]] || return 1
+    kill -TERM "$LAUNCHER_LOCK_HELPER_PID" 2>/dev/null || ! kill -0 "$LAUNCHER_LOCK_HELPER_PID" 2>/dev/null
 }
 
 release_launcher_lock() {
@@ -4051,7 +4015,7 @@ release_launcher_lock() {
         if stop_launcher_lock_helper; then
             wait "$LAUNCHER_LOCK_HELPER_PID" 2>/dev/null || true
         else
-            echo "WARN: launcher lock helper did not exit after bounded pidfd signals" >&2
+            echo "WARN: launcher lock helper did not exit after a verified release signal" >&2
         fi
     fi
     LAUNCHER_LOCK_HELPER_PID=""

tests/launcher_warm_start_recovery.sh

@@ -97,6 +97,21 @@ mkdir -p \
     "$HOME_DIR" \
     "$RUNTIME_DIR/codex-desktop"
 
+if [ "${CODEX_TEST_DISABLE_PIDFD:-0}" = "1" ]; then
+    mkdir -p "$TMP_DIR/python-site"
+    cat > "$TMP_DIR/python-site/sitecustomize.py" <<'PY'
+import os
+import signal
+
+for module, attribute in (
+    (os, "pidfd_open"),
+    (signal, "pidfd_send_signal"),
+):
+    if hasattr(module, attribute):
+        delattr(module, attribute)
+PY
+fi
+
 if [ "${CODEX_TEST_DISABLE_WARM_START:-0}" = "1" ]; then
     printf '%s\n' '{"codex-linux-warm-start-enabled":false}' \
         > "$HOME_DIR/.config/codex-desktop/settings.json"
@@ -181,6 +196,9 @@ COMMON_ENV=(
     "CODEX_WEBVIEW_PORT=$PORT"
     "CODEX_TEST_HOOK_PID_FILE=$TMP_DIR/hook.pid"
 )
+if [ "${CODEX_TEST_DISABLE_PIDFD:-0}" = "1" ]; then
+    COMMON_ENV+=("PYTHONPATH=$TMP_DIR/python-site")
+fi
 
 "${COMMON_ENV[@]}" "$APP_DIR/start.sh" > "$FIRST_LOG" 2>&1 &
 LAUNCHER_PID=$!
@@ -213,9 +231,23 @@ if [ "${CODEX_TEST_KILL_DURING_PRELAUNCH:-0}" = "1" ]; then
 fi
 
 wait_for "first Electron" pid_file_is_live
+wait_for "first launcher lock release" grep -q "electron_spawned" "$APP_LOG"
 wait_for "first packaged webview" webview_is_ready
 FIRST_ELECTRON_PID="$(read_live_app_pid)"
 
+if [ "${CODEX_TEST_NORMAL_LOCK_ONLY:-0}" = "1" ]; then
+    flock -n "$STATE_DIR/launcher.lock" true \
+        || fail "launcher lock should be released after app.pid publication"
+    if grep -q "launcher lock helper did not exit" "$FIRST_LOG"; then
+        fail "normal launcher lock release should not require pidfd escalation"
+    fi
+    kill "$FIRST_ELECTRON_PID"
+    wait "$LAUNCHER_PID"
+    LAUNCHER_PID=""
+    printf '%s\n' "launcher normal lock test passed (pidfd disabled=${CODEX_TEST_DISABLE_PIDFD:-0})"
+    exit 0
+fi
+
 kill -KILL "$LAUNCHER_PID"
 wait "$LAUNCHER_PID" 2>/dev/null || true
 LAUNCHER_PID=""

tests/scripts_smoke.sh

@@ -5580,8 +5580,13 @@ if "reap_orphaned_runtime_processes" in source or "pid_is_orphaned_runtime_proce
     raise SystemExit("lock timeout must not kill processes belonging to the active serialized cold start")
 if "LAUNCHER_LOCK_HELD=1" not in source or "stop_launcher_lock_helper" not in source:
     raise SystemExit("launcher must explicitly release and reap its dedicated lock helper")
-if "signal.SIGUSR1, 500" not in source or "signal.SIGTERM, 500" not in source or "signal.SIGKILL, 500" not in source:
-    raise SystemExit("launcher lock helper shutdown must use bounded identity-bound pidfd escalation")
+stop_helper_body = source.split("stop_launcher_lock_helper() {", 1)[1].split("release_launcher_lock() {", 1)[0]
+if "pidfd_open" in stop_helper_body or "pidfd_send_signal" in stop_helper_body:
+    raise SystemExit("normal launcher lock release must not require pidfd")
+if 'kill -TERM "$LAUNCHER_LOCK_HELPER_PID"' not in stop_helper_body:
+    raise SystemExit("launcher lock helper must release through a verified child signal")
+if 'read().strip() == "release"' in source or '"release\\n"' in source:
+    raise SystemExit("launcher lock release must not add a status-file control protocol")
 if "launcher_lock_helper_is_active" not in source or "require_active_launcher_lock" not in launch_body:
     raise SystemExit("launcher must fail closed if the identity-bound lock helper exits before Electron")
 if "LAUNCHER_LOCK_CONTROL_PATH" in source or "mkfifo" in source:
@@ -10001,6 +10006,10 @@ test_launcher_warm_start_recovery() {
     bash "$REPO_DIR/tests/launcher_warm_start_recovery.sh"
     CODEX_TEST_DISABLE_WARM_START=1 bash "$REPO_DIR/tests/launcher_warm_start_recovery.sh"
     CODEX_TEST_KILL_DURING_PRELAUNCH=1 bash "$REPO_DIR/tests/launcher_warm_start_recovery.sh"
+    CODEX_TEST_DISABLE_PIDFD=1 CODEX_TEST_NORMAL_LOCK_ONLY=1 \
+        bash "$REPO_DIR/tests/launcher_warm_start_recovery.sh"
+    CODEX_TEST_DISABLE_PIDFD=1 CODEX_TEST_KILL_DURING_PRELAUNCH=1 \
+        bash "$REPO_DIR/tests/launcher_warm_start_recovery.sh"
 }
 
 test_notification_actions_bridge_accepts_prebuilt_binary() {