From: Matteo Nastasi Date: Sun, 13 Sep 2026 13:26:29 +0000 (+0200) Subject: stop the daemon instances from piling up X-Git-Url: https://mop.ddnsfree.com/gitweb/?a=commitdiff_plain;h=ea087e53a95d39639a7c784490be7ffe6ea030c9;p=brisk.git stop the daemon instances from piling up Two defects let a second daemon start beside a running one, and once that happened the init script could not stop either of them again. pid_save() wrote its pid over whatever was in brisk.pid, and pid_remove() deleted the file without looking at whose pid was in it. So an instance exiting after another one had taken the file over left the survivor unrecorded, and from then on "stop" found no pid to kill: every restart added an orphan instead of replacing it. The orphan was not idle - a starting daemon unlinks the socket files and binds its own, so it takes every new connection while the old one stays alive on its own shared memory, and nothing says so. brisk.pid is now opened once and held under an exclusive non blocking lock for the whole life of the daemon. The kernel drops the lock when the process dies, however it dies, so neither a stale file left by a crash nor two instances starting in the same instant can get through - and checking the recorded pid for liveness could not have covered the second case. A daemon that cannot take the lock says who holds it and exits 3. pid_remove() only unlinks the file while it still holds the lock. In the init script the pipe into grep was written "\|", so it never was a pipe: the daemon was run with "|", "grep" and "IN LOOP" as three extra arguments, and the loop that restarts it watched the wrong exit status. Harmless in itself - Sac_a_push::create only looks for -d and --daemon - but the loop never worked as intended and the junk showed up in ps. "stop" now also quits our screen sessions. Killing the process alone was never enough, because each session carries the loop that respawns it; and a session left behind by a start that was refused would sit there and grab the daemon at the next stop. The loop also sleeps a second between attempts, so a daemon that cannot start does not spin. Checked in the container: stop brings a deliberately dirty state (two daemons, four screens) back to nothing; three restarts in a row leave exactly one daemon with a pid file that matches it; a second start is refused naming the holder; a kill -9 leaves the file behind and the next start takes it over anyway. 100 players with gzip afterwards: no errors, stream integrity clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE --- diff --git a/bin/brisk-init.sh b/bin/brisk-init.sh index e958057..0c29c0e 100755 --- a/bin/brisk-init.sh +++ b/bin/brisk-init.sh @@ -49,6 +49,18 @@ case "$1" in kill -KILL $pid_old 2>/dev/null || true fi fi + # + # Then close our screen sessions. Each one carries the loop that + # respawns the daemon, so killing the process alone is not enough; + # and a session left behind by a start that was refused, because the + # daemon was already running, would wait there and take the daemon + # over at the next stop. That is how the instances used to pile up. + su -s /bin/bash - ${BUSER} -c "screen -ls" 2>/dev/null \ + | sed -n "s/^[[:space:]]*\([0-9][0-9]*\.${SSUFF}\)[[:space:]].*/\1/p" \ + | while read scr ; do + su -s /bin/bash - ${BUSER} -c "screen -S $scr -X quit" >/dev/null 2>&1 || true + done + su -s /bin/bash - ${BUSER} -c "screen -wipe" >/dev/null 2>&1 || true ;; devstart) @@ -56,7 +68,7 @@ case "$1" in ;; start) - su -s /bin/bash - ${BUSER} -c 'cd '"$BPATH"'/spush ; screen -d -m -S '"${SSUFF}"' bash -c '"'"'while [ 1 ]; do cd . ; ./brisk-spush.php \| grep "IN LOOP" ; if [ $? -eq 0 ]; then break ; fi ; done'"'" + su -s /bin/bash - ${BUSER} -c 'cd '"$BPATH"'/spush ; screen -d -m -S '"${SSUFF}"' bash -c '"'"'while [ 1 ]; do cd . ; ./brisk-spush.php | grep "IN LOOP" ; if [ $? -eq 0 ]; then break ; fi ; sleep 1 ; done'"'" ;; restart) $0 stop diff --git a/web/Obj/sac-a-push.phh b/web/Obj/sac-a-push.phh index 393490b..41d122e 100644 --- a/web/Obj/sac-a-push.phh +++ b/web/Obj/sac-a-push.phh @@ -96,24 +96,60 @@ function global_dump() } +$G_pid_fp = FALSE; + function pid_save() { - $pid = getmypid(); + GLOBAL $G_pid_fp; + $fname = LEGAL_PATH."/brisk.pid"; - if (file_exists($fname)) { - log_crit("WARN: brisk.pid already exists"); + if (($fp = @fopen($fname, 'c+')) == FALSE) { + log_crit("REFUSING TO START: cannot open ".$fname); + fprintf(STDERR, "REFUSING TO START: cannot open %s\n", $fname); + return (FALSE); } - file_put_contents($fname, sprintf("%d\n", $pid)); + + /* The lock is taken for the whole life of the daemon and the kernel drops + it when the process dies, however it dies. It is the only check that + neither a stale file left by a crash nor two instances starting in the + same instant can fool, and both used to get through: the newcomer would + unlink the socket files and bind its own, taking every new connection + while the first one stayed alive, orphaned, on its own shared memory. */ + if (flock($fp, LOCK_EX | LOCK_NB) == FALSE) { + rewind($fp); + $old = intval(trim(stream_get_contents($fp))); + log_crit(sprintf("REFUSING TO START: %s is held by %d", $fname, $old)); + fprintf(STDERR, "REFUSING TO START: %s is held by %d\n", $fname, $old); + fclose($fp); + return (FALSE); + } + + ftruncate($fp, 0); + rewind($fp); + fwrite($fp, sprintf("%d\n", getmypid())); + fflush($fp); + + /* kept open on purpose: closing it would drop the lock */ + $G_pid_fp = $fp; + + return (TRUE); } function pid_remove() { - $fname = LEGAL_PATH."/brisk.pid"; + GLOBAL $G_pid_fp; - if (file_exists($fname)) { - unlink($fname); + if ($G_pid_fp == FALSE) { + /* the file was never ours: leaving it alone is what keeps the daemon + that does own it recorded, and the init script able to stop it */ + return; } + + unlink(LEGAL_PATH."/brisk.pid"); + flock($G_pid_fp, LOCK_UN); + fclose($G_pid_fp); + $G_pid_fp = FALSE; } function post_manage(&$post, $line) diff --git a/web/spush/brisk-spush.php b/web/spush/brisk-spush.php index 00d3e64..d2eb027 100755 --- a/web/spush/brisk-spush.php +++ b/web/spush/brisk-spush.php @@ -64,7 +64,9 @@ function main($argv) { GLOBAL $G_ban_list, $G_black_list, $G_cloud_smasher, $G_provider_proxy; - pid_save(); + if (pid_save() == FALSE) { + exit(3); + } do { if (($brisk = Brisk::create(LEGAL_PATH."/brisk-crystal.data", $G_ban_list, $G_black_list, $G_cloud_smasher)) == FALSE) { log_crit("Brisk::create failed");