Skip to content

Commit 0597158

Browse files
fix(docker): bound the shutdown wait so a stuck process cannot strand PID 1
stop_process_pid signalled each managed process and then waited on it forever. A process that refuses to exit therefore wedges PID 1, and the container sits "Up" serving nothing. No restart policy can detect that, because policies only react to exits. Valkey reaches exactly that state: it aborts its own shutdown when the snapshot it writes on exit cannot be written, so on a read-only /redis-data it catches SIGTERM and keeps running. Because the init script traps EXIT, every fatal error path runs shutdown(), including a failed startup migration when the database is not reachable yet. The daemon's restart policy does not honour compose's depends_on at host boot, so RomM regularly starts before its database and takes that path. The result is permanent downtime that needs a manual restart. Wait STOP_TIMEOUT (default 10s) for a signalled process, then escalate to SIGKILL, with a second bounded wait and a warning if even that fails. The failing path now ends in an exit, which a restart policy can act on. Add a healthcheck to the compose example as well, since a hung PID 1 is otherwise invisible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent ff8d4d0 commit 0597158

4 files changed

Lines changed: 50 additions & 9 deletions

File tree

docker/init_scripts/init

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ REDIS_HOST="${REDIS_HOST:=""}"
2323
# hour of sessions and cached metadata. Set to an empty value to never snapshot.
2424
REDIS_SAVE_POLICY="${REDIS_SAVE_POLICY="3600 1"}"
2525

26+
# how long to wait for a signalled process to exit before escalating to SIGKILL
27+
STOP_TIMEOUT="${STOP_TIMEOUT:="10"}"
28+
2629
# logger colors
2730
RED='\033[0;31m'
2831
LIGHTMAGENTA='\033[0;95m'
@@ -305,15 +308,39 @@ watchdog_process_pid() {
305308
fi
306309
}
307310

311+
# Wait for PID $1 to disappear, polling every 100ms for at most $2 polls.
312+
# Returns non-zero if it is still alive when the budget runs out.
313+
wait_for_pid_exit() {
314+
local pid=$1 max_polls=$2 polls=0
315+
while [[ -e "/proc/${pid}" ]]; do
316+
if ((polls >= max_polls)); then
317+
return 1
318+
fi
319+
sleep 0.1
320+
polls=$((polls + 1))
321+
done
322+
return 0
323+
}
324+
308325
stop_process_pid() {
309326
PROCESS=$1
310327
if [[ -f "/tmp/${PROCESS}.pid" ]]; then
311328
PID=$(cat "/tmp/${PROCESS}.pid") || true
312329
if [[ -d "/proc/${PID}" ]]; then
313330
info_log "Stopping ${PROCESS}"
314331
kill "${PID}" || true
315-
# wait for process exit
316-
while [[ -e "/proc/${PID}" ]]; do sleep 0.1; done
332+
333+
# A process that will not exit must never strand PID 1. Valkey aborts
334+
# its own shutdown when the snapshot it writes on exit fails, so on a
335+
# read-only /redis-data an unbounded wait here leaves the container up
336+
# and serving nothing, which no restart policy can detect.
337+
if ! wait_for_pid_exit "${PID}" $((STOP_TIMEOUT * 10)); then
338+
warn_log "${PROCESS} did not exit within ${STOP_TIMEOUT}s, sending SIGKILL"
339+
kill -9 "${PID}" || true
340+
if ! wait_for_pid_exit "${PID}" $((STOP_TIMEOUT * 10)); then
341+
warn_log "${PROCESS} survived SIGKILL, continuing shutdown without it"
342+
fi
343+
fi
317344
fi
318345
fi
319346
}

docs/BACKEND_ARCHITECTURE.md

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,13 +1516,14 @@ Falls back to `FakeRedis` in test mode.
15161516

15171517
#### Core
15181518

1519-
| Variable | Default | Description |
1520-
| ---------------- | ---------------- | -------------------- |
1521-
| `ROMM_BASE_PATH` | `/romm` | Base data directory |
1522-
| `ROMM_BASE_URL` | `http://0.0.0.0` | Application base URL |
1523-
| `ROMM_PORT` | `8080` | Server port |
1524-
| `DEV_MODE` | `false` | Development mode |
1525-
| `LOGLEVEL` | `INFO` | Log level |
1519+
| Variable | Default | Description |
1520+
| ---------------- | ---------------- | ---------------------------------- |
1521+
| `ROMM_BASE_PATH` | `/romm` | Base data directory |
1522+
| `ROMM_BASE_URL` | `http://0.0.0.0` | Application base URL |
1523+
| `ROMM_PORT` | `8080` | Server port |
1524+
| `DEV_MODE` | `false` | Development mode |
1525+
| `LOGLEVEL` | `INFO` | Log level |
1526+
| `STOP_TIMEOUT` | `10` | Seconds before SIGKILL on shutdown |
15261527

15271528
#### Database
15281529

env.template

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ ROMM_TMP_PATH= # Custom temporary directory path
44
ROMM_BASE_URL=http://0.0.0.0 # Public URL of this instance
55
ROMM_PORT=8080 # Port on which the application listens
66
KIOSK_MODE=false # Read-only mode for public displays or kiosks
7+
STOP_TIMEOUT=10 # Seconds to wait for a service to exit on shutdown before sending SIGKILL
78

89
# Database
910
ROMM_DB_DRIVER=mariadb # Database driver to use (mariadb, mysql, postgresql)

examples/docker-compose.example.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,18 @@ services:
2727
- /path/to/config:/romm/config # (Optional) Path where config.yml is stored
2828
ports:
2929
- 80:8080
30+
healthcheck:
31+
# $$ defers expansion to the container, so a custom ROMM_PORT still works
32+
test:
33+
[
34+
"CMD-SHELL",
35+
"curl -fsS http://localhost:$${ROMM_PORT:-8080}/api/heartbeat",
36+
]
37+
start_period: 60s
38+
start_interval: 10s
39+
interval: 30s
40+
timeout: 5s
41+
retries: 3
3042
depends_on:
3143
romm-db:
3244
condition: service_healthy

0 commit comments

Comments
 (0)