Browse Source

fix: no daemon request may hang the dispatcher — Soup session timeouts (SADDLE_HTTP_TIMEOUT_S, default 30s, idle-pool reaping) plus a 60s tick watchdog that force-resets the latch; request paths already retry transport blips once

clanker 1 month ago
parent
commit
c6af578198
3 changed files with 78 additions and 3 deletions
  1. 16 2
      src/Services/Dispatcher.vala
  2. 19 1
      src/Services/SaddleService.vala
  3. 43 0
      test/web_stack_test.py

+ 16 - 2
src/Services/Dispatcher.vala

@@ -19,6 +19,7 @@ namespace ClankerSaddle.Services {
 
         private Container container = inject<Container>();
         private bool ticking = false;
+        private int64 tick_started_ms = 0;
         private bool warned_no_llm = false;
 
         /**
@@ -47,16 +48,29 @@ namespace ClankerSaddle.Services {
         }
 
         private async void run_tick() {
+            int64 now_ms = get_monotonic_time() / 1000;
             if (ticking) {
-                return;
+                // Watchdog: a tick stuck longer than 60s (an await hung
+                // past the Soup timeout, or anything else) must not
+                // silence the whole system — force-reset the latch and
+                // proceed. A frozen tick is the single failure that stops
+                // every dispatch; overlapping a zombie tick is the lesser
+                // evil.
+                if (now_ms - tick_started_ms < 60000) {
+                    return;
+                }
+                warning("[ClankerSaddle] Dispatch tick exceeded 60s; resetting the latch"
+                    + " (a previous tick may still be awaiting)");
             }
             ticking = true;
+            tick_started_ms = now_ms;
             try {
                 yield tick();
             } catch (Error e) {
                 warning("[ClankerSaddle] Dispatch tick failed: %s", e.message);
+            } finally {
+                ticking = false;
             }
-            ticking = false;
         }
 
         private int strike_count(int64 saddle_id) {

+ 19 - 1
src/Services/SaddleService.vala

@@ -71,7 +71,25 @@ namespace ClankerSaddle.Services {
     public class SaddleService : Object {
 
         private OrmSession db = inject<OrmSession>();
-        private Soup.Session http = new Soup.Session();
+        /**
+         * Every daemon request shares this session and therefore its
+         * timeouts: without them a single hung request (a daemon wedged
+         * mid-response over the keep-alive pool) froze the dispatcher's
+         * tick latch forever. `timeout` bounds each request
+         * (SADDLE_HTTP_TIMEOUT_S, default 30s); `idle_timeout` reaps
+         * pooled connections so a peer that vanished between requests is
+         * not dialed again.
+         */
+        private Soup.Session http = new Soup.Session() {
+            timeout = http_timeout_seconds(),
+            idle_timeout = 10
+        };
+
+        private static uint http_timeout_seconds() {
+            var raw = Environment.get_variable("SADDLE_HTTP_TIMEOUT_S") ?? "";
+            int64 parsed = int64.parse(raw);
+            return parsed > 0 ? (uint) parsed : 30;
+        }
         private SaddlePollState poll_state = inject<SaddlePollState>();
         private StatePulse pulse = inject<StatePulse>();
         private RuleService rules = inject<RuleService>();

+ 43 - 0
test/web_stack_test.py

@@ -214,6 +214,7 @@ class FakeSaddle:
         self.output_seq = 100
         self.subtask_status_events = []  # recorded POST /subtask-status payloads
         self.fail_subtask_status = False  # exercise the web's retry path
+        self.hang_next = {}              # path suffix -> hang count (never responds)
 
     def set_state(self, state, summary=None):
         """Advances the scripted lifecycle.
@@ -261,6 +262,15 @@ class FakeSaddle:
         def log_message(self, fmt, *args):
             pass
 
+        def maybe_hang(self, path):
+            """True when this request should hang forever (never respond),
+            consuming one unit of the path's hang budget."""
+            for suffix, count in list(self.daemon.hang_next.items()):
+                if count > 0 and path.startswith(suffix):
+                    self.daemon.hang_next[suffix] = count - 1
+                    return True
+            return False
+
         def _send(self, status, obj):
             body = json.dumps(obj).encode()
             self.send_response(status)
@@ -309,6 +319,9 @@ class FakeSaddle:
                 self._send(404, {"error": "no such route"})
 
         def do_GET(self):
+            if self.maybe_hang(self.path):
+                time.sleep(600)
+                return
             if self.path == "/status":
                 response = {"state": self.daemon.state, "summary": self.daemon.summary}
                 if self.daemon.resumed_pending:
@@ -369,6 +382,7 @@ def start_app():
         "SPRY_SEED_ADMIN_PASSWORD": "adminpass123",
         "SADDLE_LLM_BASE_URL": "http://127.0.0.1:%d" % DAEMON_PORT,
         "SADDLE_PROJECTS_ROOT": os.path.join(WORK, "workspace"),
+        "SADDLE_HTTP_TIMEOUT_S": "5",
         # Never inherit a caller-poisoned LD_LIBRARY_PATH (e.g. one that
         # resolves the ancient /lib64 copies): empty makes the loader use
         # the binary's RUNPATH, which is the dev stack it was built
@@ -1768,6 +1782,35 @@ def main():
         status, body = saddle_api("GET", "/api/saddle/task/99999", r4_token)
         check(status == 404, "unknown task id returns 404")
 
+
+        print("== R6 F1: a hanging request cannot freeze the tick ==")
+        # One wedged /output (never responds) blocks a poll; the Soup
+        # timeout must fail it and later ticks keep dispatching.
+        task_state(auth, lineage_task)
+        conn = db()
+        conn.execute("insert into tasks (project_id, title, status, priority, created_at)"
+                     " values (1, 'Hang survivor', 'queued', 2, strftime('%s','now'))")
+        hang_task = conn.execute("select max(id) from tasks").fetchone()[0]
+        conn.close()
+        fake.hang_next["/output"] = 1
+        # Dispatch a task first so a poll is in flight against the hang,
+        # then verify a later task still dispatches (tick not frozen).
+        act(auth, slot_public(auth, "/tasks", "tasks")["create"], {
+            "project_id": "1",
+            "title": "Hang runner",
+            "priority": "3",
+        })
+        hang_runner = int([t["id"] for t in slot_public(auth, "/tasks", "tasks")["tasks"]
+                           if t["title"] == "Hang runner"][0])
+        check(wait_task_done(auth, hang_runner, timeout=90) is not None,
+              "dispatching task survives a hung daemon request")
+        # The hung poll itself fails after the timeout; its task requeues
+        # or completes on later ticks — the system kept moving either way.
+        with open(os.path.join(WORK, "app.log")) as handle:
+            app_log = handle.read()
+        check("tick exceeded" not in app_log or "Dispatch tick failed" in app_log,
+              "a hung request never freezes the tick latch")
+
     finally:
         # Exit-code diagnostic: distinguishes a crashed app (negative
         # returncode, e.g. -11 SIGSEGV) from a clean shutdown. When