From 1900ec0949c92539c8b7779672a838b53c899b38 Mon Sep 17 00:00:00 2001 From: Cigydd Date: Tue, 16 Jun 2026 09:51:22 +0200 Subject: [PATCH] AsyncTask: fix double-unref crash on teardown after completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two reader threads (read_stdout/read_stderr) cleared their stdout_is_open/stderr_is_open flags *before* closing and releasing their DataInputStream. Whichever reader finished first could therefore see both flags clear and call finish() — signalling completion (status = FINISHED, task_complete()) — while the peer thread was still inside dis_*.close() / dis_* = null. The owner, seeing the task finished, then tore it down while a reader was still unreffing its stream, double- unreffing the underlying UnixInputStream and crashing in g_object_unref (SIGSEGV during teardown, observed sporadically from `--check --scripted` cron runs). The reader threads were also created detached and never joined, so a reader could outlive completion signalling entirely. Fix: keep handles to both reader threads and add a small coordinator thread (wait_and_finish) that joins both readers — guaranteeing their streams are fully closed — before calling finish() exactly once. Remove the racy stdout_is_open/stderr_is_open flags and the finish() calls from the reader threads. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Utility/AsyncTask.vala | 56 ++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/src/Utility/AsyncTask.vala b/src/Utility/AsyncTask.vala index e9f9421c..329a8707 100644 --- a/src/Utility/AsyncTask.vala +++ b/src/Utility/AsyncTask.vala @@ -37,8 +37,8 @@ public abstract class AsyncTask : GLib.Object{ private DataInputStream dis_err = null; protected DataOutputStream dos_log = null; - private bool stdout_is_open = false; - private bool stderr_is_open = false; + private Thread stdout_thread = null; + private Thread stderr_thread = null; protected Pid child_pid = 0; private int input_fd = -1; @@ -166,7 +166,7 @@ public abstract class AsyncTask : GLib.Object{ try { //start thread for reading output stream - new Thread.try ("async-task-stdout-reader", read_stdout); + stdout_thread = new Thread.try ("async-task-stdout-reader", read_stdout); } catch (GLib.Error e) { log_error ("AsyncTask.begin():create_thread:read_stdout()"); log_error (e.message); @@ -174,11 +174,25 @@ public abstract class AsyncTask : GLib.Object{ try { //start thread for reading error stream - new Thread.try ("async-task-stderr-reader", read_stderr); + stderr_thread = new Thread.try ("async-task-stderr-reader", read_stderr); } catch (GLib.Error e) { log_error ("AsyncTask.begin():create_thread:read_stderr()"); log_error (e.message); } + + try { + // Wait for both reader threads to finish (streams fully closed), + // then signal completion exactly once. Previously each reader called + // finish() on its own as soon as both *_is_open flags were clear, but + // those flags were cleared *before* the stream was actually closed. + // That let one reader fire finish()/task_complete() while the other was + // still inside dis_*.close()/= null; the owner would then tear the task + // down mid-unref, double-unreffing the underlying stream (SIGSEGV). + new Thread.try ("async-task-finish-waiter", wait_and_finish); + } catch (GLib.Error e) { + log_error ("AsyncTask.begin():create_thread:wait_and_finish()"); + log_error (e.message); + } } catch (Error e) { log_error ("AsyncTask.begin()"); @@ -192,8 +206,6 @@ public abstract class AsyncTask : GLib.Object{ private void read_stdout() { try { - stdout_is_open = true; - out_line = dis_out.read_line (null); while (out_line != null) { //log_msg("O: " + out_line); @@ -204,8 +216,6 @@ public abstract class AsyncTask : GLib.Object{ out_line = dis_out.read_line (null); //read next } - stdout_is_open = false; - // dispose stdout try { if (dis_out != null) { @@ -214,22 +224,15 @@ public abstract class AsyncTask : GLib.Object{ } catch (GLib.Error ignored) {} dis_out = null; - - // check if complete - if (!stdout_is_open && !stderr_is_open){ - finish(); - } } catch (Error e) { log_error ("AsyncTask.read_stdout()"); log_error (e.message); } } - + private void read_stderr() { try { - stderr_is_open = true; - err_line = dis_err.read_line (null); while (err_line != null) { if (err_line.length > 0){ @@ -239,8 +242,6 @@ public abstract class AsyncTask : GLib.Object{ err_line = dis_err.read_line (null); //read next } - stderr_is_open = false; - // dispose stderr try { if (dis_err != null) { @@ -249,11 +250,6 @@ public abstract class AsyncTask : GLib.Object{ } catch (GLib.Error ignored) {} dis_err = null; - - // check if complete - if (!stdout_is_open && !stderr_is_open){ - finish(); - } } catch (Error e) { log_error ("AsyncTask.read_stderr()"); @@ -261,6 +257,20 @@ public abstract class AsyncTask : GLib.Object{ } } + // Joins both reader threads (so their streams are fully closed) and then runs + // finish() exactly once, from a single thread. See begin() for the rationale. + private void wait_and_finish() { + if (stdout_thread != null) { + stdout_thread.join(); + stdout_thread = null; + } + if (stderr_thread != null) { + stderr_thread.join(); + stderr_thread = null; + } + finish(); + } + public void write_stdin(string line){ try{ if (status == AppStatus.RUNNING){