[projects/buildlogs] Reap the decompressor and detect aborted clients in dump_log/dump_text.

[arekm]

commit c54450c17c6fa4b5584fbbb51f589e7809508a04
Author: Arkadiusz Miśkiewicz <arekm at maven.pl>
Date:   Mon May 4 18:56:11 2026 +0200

    Reap the decompressor and detect aborted clients in dump_log/dump_text.
    
    Long-running requests were leaving lbzcat children chewing CPU and
    piling up as zombies when the client disconnected or PHP died unhappily.
    Switch to single-threaded bzcat, terminate the proc on close, register
    a shutdown hook so cleanup runs on any exit path, and add a periodic
    heartbeat in the read loop so connection_aborted() can trip mid-stream.

 index.php | 36 ++++++++++++++++++++++++++++++++++++
 lib.php   | 14 +++++++++-----
 2 files changed, 45 insertions(+), 5 deletions(-)
---
diff --git a/index.php b/index.php
index 964f1ae..749b6a8 100644
--- a/index.php
+++ b/index.php
@@ -801,6 +801,8 @@ function dump_log($tail)
 	global $root_directory, $big_url, $ns, $id, $cnt, $off;
 	global $buildlogs_server;
 
+	ignore_user_abort(false);
+
 	$f = file_name();
 
 	if ($f == false)
@@ -893,6 +895,17 @@ function dump_log($tail)
 		echo "</table>";
 		return;
 	}
+	// Make sure the decompressor child gets reaped (and killed) even if PHP
+	// dies on a fatal, max_execution_time, or unnoticed client abort. Without
+	// this, lbzcat keeps burning CPU and accumulates as a zombie. Guarded with
+	// is_resource() because close_log_stream() may have already run cleanly,
+	// and proc_terminate/proc_close on a freed resource throws TypeError in
+	// PHP 8 (which @ does not suppress).
+	register_shutdown_function(function() use ($h) {
+		if (is_resource($h['proc'])) { @proc_terminate($h['proc'], 15); }
+		if (is_resource($h['fd']))   { @fclose($h['fd']); }
+		if (is_resource($h['proc'])) { @proc_close($h['proc']); }
+	});
 	$fd = $h['fd'];
 	$toc = array();
 	$err = array();
@@ -903,7 +916,21 @@ function dump_log($tail)
 	$out_buf_size = 0;
 	$err_count = 0;
 	$seen_sections = array();
+	$tick = 0;
 	while (($s = fgets($fd, 102400)) != false) {
+		// Periodically poke the socket so PHP notices if the client is gone.
+		// Best-effort: with mod_deflate or PHP buffering this may not actually
+		// reach the wire mid-loop, but the shutdown hook is the real safety
+		// net — this just lets us bail early when it does work.
+		if ((++$tick & 0x7FF) === 0) {
+			echo "<!-- . -->";
+			@ob_flush();
+			@flush();
+			if (connection_aborted()) {
+				close_log_stream($h);
+				return;
+			}
+		}
 
 		$toc_elem = false;
 		$err_elem = false;
@@ -1058,6 +1085,15 @@ function dump_text()
 		echo "# failed to spawn decompressor\n";
 		return;
 	}
+	// Reap the decompressor on PHP fatal / timeout / late-noticed abort so we
+	// don't accumulate lbzcat zombies still chewing on the file. See dump_log
+	// for the is_resource() rationale (PHP 8 TypeError on freed resource).
+	register_shutdown_function(function() use ($h) {
+		if (is_resource($h['proc'])) { @proc_terminate($h['proc'], 15); }
+		if (is_resource($h['fd']))   { @fclose($h['fd']); }
+		if (is_resource($h['proc'])) { @proc_close($h['proc']); }
+	});
+	ignore_user_abort(false);
 	fpassthru($h['fd']);
 	close_log_stream($h);
 }
diff --git a/lib.php b/lib.php
index b67f217..6fd3edc 100644
--- a/lib.php
+++ b/lib.php
@@ -9,9 +9,7 @@ function open_db(string $dsn): PDO {
 }
 
 function decompressor_for(string $path): string {
-    if (str_ends_with($path, '.bz2')) {
-        return is_executable('/usr/bin/lbzcat') ? '/usr/bin/lbzcat' : '/usr/bin/bzcat';
-    }
+    if (str_ends_with($path, '.bz2')) return '/usr/bin/bzcat';
     if (str_ends_with($path, '.gz'))  return '/usr/bin/zcat';
     if (str_ends_with($path, '.xz'))  return '/usr/bin/xzcat';
     if (str_ends_with($path, '.zst')) return '/usr/bin/zstdcat';
@@ -31,8 +29,14 @@ function open_log_stream(string $path) {
 }
 
 function close_log_stream(array $h): void {
-    fclose($h['fd']);
-    proc_close($h['proc']);
+    // Kill the decompressor first so proc_close doesn't block waiting on it
+    // when we abandoned the read early (client aborted, error truncation, etc.).
+    // is_resource() guards because PHP 8 throws TypeError (not a warning) when
+    // these are called on an already-closed handle, so a double-close — e.g.
+    // close + shutdown hook — would otherwise blow up.
+    if (is_resource($h['proc'])) { @proc_terminate($h['proc'], 15); }
+    if (is_resource($h['fd']))   { @fclose($h['fd']); }
+    if (is_resource($h['proc'])) { @proc_close($h['proc']); }
 }
 
 function format_size(int $bytes): string {
================================================================

---- gitweb:

http://git.pld-linux.org/gitweb.cgi/projects/buildlogs.git/commitdiff/c54450c17c6fa4b5584fbbb51f589e7809508a04