Skip to content

Commit 7cc47e5

Browse files
committed
fix(launcher): support Python without pidfd APIs
1 parent 6a9ccd6 commit 7cc47e5

4 files changed

Lines changed: 56 additions & 48 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
3535

3636
### Fixed
3737

38+
- Launcher startup no longer requires Python's pidfd wrappers for normal
39+
launcher lock acquire and release. Pidfd remains reserved for the
40+
identity-verified stale Electron termination path.
3841
- Approval notifications now preserve the upstream Approve, Approve for
3942
session, and Decline actions on Linux. A small freedesktop notification
4043
bridge forwards the action and close signals that Electron's Linux

launcher/start.sh.template

Lines changed: 10 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -3873,9 +3873,6 @@ parent_pid = int(sys.argv[1])
38733873
lock_path, wait_text, status_path = sys.argv[2:]
38743874
wait_seconds = int(wait_text)
38753875

3876-
if not hasattr(os, "pidfd_open") or not hasattr(signal, "pidfd_send_signal"):
3877-
sys.exit(1)
3878-
38793876
libc_name = ctypes.util.find_library("c") or "libc.so.6"
38803877
try:
38813878
libc = ctypes.CDLL(libc_name, use_errno=True)
@@ -3978,49 +3975,16 @@ launcher_lock_helper_is_active() {
39783975
}
39793976

39803977
stop_launcher_lock_helper() {
3981-
python3 - \
3982-
"$LAUNCHER_LOCK_HELPER_PID" \
3983-
"$LAUNCHER_LOCK_HELPER_START_TIME" \
3984-
"${BASHPID:-$$}" <<'PY'
3985-
import os
3986-
import select
3987-
import signal
3988-
import sys
3989-
3990-
pid = int(sys.argv[1])
3991-
expected_start_time = sys.argv[2]
3992-
expected_parent = int(sys.argv[3])
3978+
local actual_start_time
3979+
local cmdline
39933980

3994-
if not hasattr(os, "pidfd_open") or not hasattr(signal, "pidfd_send_signal"):
3995-
sys.exit(1)
3996-
try:
3997-
pidfd = os.pidfd_open(pid, 0)
3998-
with open(f"/proc/{pid}/stat", "r", encoding="utf-8") as handle:
3999-
fields = handle.read().rsplit(") ", 1)[1].split()
4000-
if fields[19] != expected_start_time or int(fields[1]) != expected_parent:
4001-
os.close(pidfd)
4002-
sys.exit(1)
4003-
poller = select.poll()
4004-
poller.register(pidfd, select.POLLIN)
4005-
exited = False
4006-
for requested_signal, wait_ms in (
4007-
(signal.SIGUSR1, 500),
4008-
(signal.SIGTERM, 500),
4009-
(signal.SIGKILL, 500),
4010-
):
4011-
try:
4012-
signal.pidfd_send_signal(pidfd, requested_signal)
4013-
except ProcessLookupError:
4014-
exited = True
4015-
break
4016-
if poller.poll(wait_ms):
4017-
exited = True
4018-
break
4019-
os.close(pidfd)
4020-
sys.exit(0 if exited else 1)
4021-
except (OSError, IndexError, ValueError):
4022-
sys.exit(1)
4023-
PY
3981+
[[ "$LAUNCHER_LOCK_HELPER_PID" =~ ^[0-9]+$ ]] || return 1
3982+
actual_start_time="$(pid_start_time "$LAUNCHER_LOCK_HELPER_PID" 2>/dev/null || true)"
3983+
[ -n "$actual_start_time" ] && [ "$actual_start_time" = "$LAUNCHER_LOCK_HELPER_START_TIME" ] || return 1
3984+
pid_parent_matches "$LAUNCHER_LOCK_HELPER_PID" "${BASHPID:-$$}" || return 1
3985+
cmdline="$(pid_cmdline "$LAUNCHER_LOCK_HELPER_PID")"
3986+
[[ "$cmdline" == *" $APP_STATE_DIR/launcher.lock "*" $LAUNCHER_LOCK_STATUS_PATH"* ]] || return 1
3987+
kill -TERM "$LAUNCHER_LOCK_HELPER_PID" 2>/dev/null || ! kill -0 "$LAUNCHER_LOCK_HELPER_PID" 2>/dev/null
40243988
}
40253989

40263990
release_launcher_lock() {
@@ -4030,7 +3994,7 @@ release_launcher_lock() {
40303994
if stop_launcher_lock_helper; then
40313995
wait "$LAUNCHER_LOCK_HELPER_PID" 2>/dev/null || true
40323996
else
4033-
echo "WARN: launcher lock helper did not exit after bounded pidfd signals" >&2
3997+
echo "WARN: launcher lock helper did not exit after a verified release signal" >&2
40343998
fi
40353999
fi
40364000
LAUNCHER_LOCK_HELPER_PID=""

tests/launcher_warm_start_recovery.sh

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,21 @@ mkdir -p \
9797
"$HOME_DIR" \
9898
"$RUNTIME_DIR/codex-desktop"
9999

100+
if [ "${CODEX_TEST_DISABLE_PIDFD:-0}" = "1" ]; then
101+
mkdir -p "$TMP_DIR/python-site"
102+
cat > "$TMP_DIR/python-site/sitecustomize.py" <<'PY'
103+
import os
104+
import signal
105+
106+
for module, attribute in (
107+
(os, "pidfd_open"),
108+
(signal, "pidfd_send_signal"),
109+
):
110+
if hasattr(module, attribute):
111+
delattr(module, attribute)
112+
PY
113+
fi
114+
100115
if [ "${CODEX_TEST_DISABLE_WARM_START:-0}" = "1" ]; then
101116
printf '%s\n' '{"codex-linux-warm-start-enabled":false}' \
102117
> "$HOME_DIR/.config/codex-desktop/settings.json"
@@ -182,6 +197,9 @@ COMMON_ENV=(
182197
"CODEX_WEBVIEW_PORT=$PORT"
183198
"CODEX_TEST_HOOK_PID_FILE=$TMP_DIR/hook.pid"
184199
)
200+
if [ "${CODEX_TEST_DISABLE_PIDFD:-0}" = "1" ]; then
201+
COMMON_ENV+=("PYTHONPATH=$TMP_DIR/python-site")
202+
fi
185203

186204
"${COMMON_ENV[@]}" "$APP_DIR/start.sh" > "$FIRST_LOG" 2>&1 &
187205
LAUNCHER_PID=$!
@@ -214,9 +232,23 @@ if [ "${CODEX_TEST_KILL_DURING_PRELAUNCH:-0}" = "1" ]; then
214232
fi
215233

216234
wait_for "first Electron" pid_file_is_live
235+
wait_for "first launcher lock release" grep -q "electron_spawned" "$APP_LOG"
217236
wait_for "first packaged webview" webview_is_ready
218237
FIRST_ELECTRON_PID="$(read_live_app_pid)"
219238

239+
if [ "${CODEX_TEST_NORMAL_LOCK_ONLY:-0}" = "1" ]; then
240+
flock -n "$STATE_DIR/launcher.lock" true \
241+
|| fail "launcher lock should be released after app.pid publication"
242+
if grep -q "launcher lock helper did not exit" "$FIRST_LOG"; then
243+
fail "normal launcher lock release should not require pidfd escalation"
244+
fi
245+
kill "$FIRST_ELECTRON_PID"
246+
wait "$LAUNCHER_PID"
247+
LAUNCHER_PID=""
248+
printf '%s\n' "launcher normal lock test passed (pidfd disabled=${CODEX_TEST_DISABLE_PIDFD:-0})"
249+
exit 0
250+
fi
251+
220252
kill -KILL "$LAUNCHER_PID"
221253
wait "$LAUNCHER_PID" 2>/dev/null || true
222254
LAUNCHER_PID=""

tests/scripts_smoke.sh

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5587,8 +5587,13 @@ if "reap_orphaned_runtime_processes" in source or "pid_is_orphaned_runtime_proce
55875587
raise SystemExit("lock timeout must not kill processes belonging to the active serialized cold start")
55885588
if "LAUNCHER_LOCK_HELD=1" not in source or "stop_launcher_lock_helper" not in source:
55895589
raise SystemExit("launcher must explicitly release and reap its dedicated lock helper")
5590-
if "signal.SIGUSR1, 500" not in source or "signal.SIGTERM, 500" not in source or "signal.SIGKILL, 500" not in source:
5591-
raise SystemExit("launcher lock helper shutdown must use bounded identity-bound pidfd escalation")
5590+
stop_helper_body = source.split("stop_launcher_lock_helper() {", 1)[1].split("release_launcher_lock() {", 1)[0]
5591+
if "pidfd_open" in stop_helper_body or "pidfd_send_signal" in stop_helper_body:
5592+
raise SystemExit("normal launcher lock release must not require pidfd")
5593+
if 'kill -TERM "$LAUNCHER_LOCK_HELPER_PID"' not in stop_helper_body:
5594+
raise SystemExit("launcher lock helper must release through a verified child signal")
5595+
if 'read().strip() == "release"' in source or '"release\\n"' in source:
5596+
raise SystemExit("launcher lock release must not add a status-file control protocol")
55925597
if "launcher_lock_helper_is_active" not in source or "require_active_launcher_lock" not in launch_body:
55935598
raise SystemExit("launcher must fail closed if the identity-bound lock helper exits before Electron")
55945599
if "LAUNCHER_LOCK_CONTROL_PATH" in source or "mkfifo" in source:
@@ -10078,6 +10083,10 @@ test_launcher_warm_start_recovery() {
1007810083
bash "$REPO_DIR/tests/launcher_warm_start_recovery.sh"
1007910084
CODEX_TEST_DISABLE_WARM_START=1 bash "$REPO_DIR/tests/launcher_warm_start_recovery.sh"
1008010085
CODEX_TEST_KILL_DURING_PRELAUNCH=1 bash "$REPO_DIR/tests/launcher_warm_start_recovery.sh"
10086+
CODEX_TEST_DISABLE_PIDFD=1 CODEX_TEST_NORMAL_LOCK_ONLY=1 \
10087+
bash "$REPO_DIR/tests/launcher_warm_start_recovery.sh"
10088+
CODEX_TEST_DISABLE_PIDFD=1 CODEX_TEST_KILL_DURING_PRELAUNCH=1 \
10089+
bash "$REPO_DIR/tests/launcher_warm_start_recovery.sh"
1008110090
}
1008210091

1008310092
test_notification_actions_bridge_accepts_prebuilt_binary() {

0 commit comments

Comments
 (0)