fix(net): honor Ctrl-C during a retry backoff

scripts/regressions/retry-backoff-sleeps-off-ctrl-c.sh

@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+# Regression: a Ctrl-C arriving during an HTTP retry backoff must end the wait,
+# not be slept off and followed by another re-dial.
+#
+# The bug: HttpClient.retrySleep - the one backoff shared by all four retry
+# loops - read cancellation only off std.Io.sleep's error.Canceled. Nothing in
+# this process ever arms that channel, while the live stop signal is the
+# `cancel` predicate main wires to the interrupt flag. So a cancel landing
+# during a backoff was ignored for the whole schedule (up to 1s + 2s + 4s) and
+# the request was re-dialled anyway. The doc comment claimed the opposite,
+# which is what let it survive.
+#
+# The fix polls `cancel` before the wait and between short slices of it, so the
+# stop is honoured promptly. The error.Canceled return plumbing already existed
+# at every call site; only the trigger was dead.
+#
+# Driven through the inline unit tests: retrySleep is private and the contract
+# is about a predicate plus a clock, so no registry or network is needed. Grep
+# guards first so a deleted poll cannot go green vacuously.
+
+set -euo pipefail
+
+ROOT=$(cd "$(dirname "$0")/../.." && pwd)
+cd "$ROOT"
+
+SRC="src/net/client.zig"
+
+# Both halves must survive: the predicate poll inside the backoff, and the
+# sliced wait that lets a mid-backoff cancel be seen at all.
+if ! grep -Fqs -- "backoff_poll_slice_ms" "$SRC"; then
+  echo "FAIL: the retry backoff no longer waits in cancellable slices" >&2
+  exit 1
+fi
+if ! grep -Fqs -- "retrySleep observes a cancel that arrives mid-backoff" "$SRC"; then
+  echo "FAIL: the mid-backoff cancellation test is missing" >&2
+  exit 1
+fi
+
+BIN="$ROOT/zig-out/test-bin/lib_tests"
+# Always rebuild so the binary reflects current source; zig's cache keeps a
+# no-op rebuild cheap.
+if ! zig build test-bin >/dev/null 2>&1; then
+  echo "FAIL: could not build the unit test binary (zig build test-bin)" >&2
+  exit 1
+fi
+
+OUT=$("$BIN" 2>&1) && STATUS=0 || STATUS=$?
+if [[ "$STATUS" -ne 0 ]]; then
+  echo "FAIL: a cancel during a retry backoff was slept off" >&2
+  printf '%s\n' "$OUT" | grep -iE "FAIL \(|[0-9]+ failed|expected .* found|leaked" >&2 || true
+  exit 1
+fi
+
+echo "PASS: a Ctrl-C during a retry backoff ends the wait instead of re-dialling"

src/net/client.zig

@@ -1033,17 +1033,31 @@ pub const HttpClient = struct {
         return true;
     }
 
-    /// Cancellable backoff between retry attempts. Common to every
-    /// retry loop so a Ctrl-C during the wait surfaces immediately
-    /// instead of being swallowed by std.Io.sleep's silent return.
+    /// How long the backoff may nap before re-checking `cancel`. Short
+    /// enough that a Ctrl-C is honoured promptly, long enough that a 4 s
+    /// wait is not thousands of wakeups.
+    const backoff_poll_slice_ms: u64 = 50;
+
+    /// Cancellable backoff between retry attempts, shared by every retry
+    /// loop. std.Io cancellation is never armed in this process, so the wait
+    /// polls `cancel` itself - trusting `std.Io.sleep` alone would sleep a
+    /// Ctrl-C off and re-dial.
     fn retrySleep(self: *HttpClient, attempt: usize) error{Canceled}!void {
-        std.Io.sleep(
-            self.io,
-            std.Io.Duration.fromNanoseconds(@intCast(self.retry_backoff_ms[attempt] * std.time.ns_per_ms)),
-            .awake,
-        ) catch |e| switch (e) {
-            error.Canceled => return error.Canceled,
-        };
+        const total_ms = self.retry_backoff_ms[attempt];
+        var waited: u64 = 0;
+        while (true) {
+            if (self.cancel) |cancelled| if (cancelled()) return error.Canceled;
+            if (waited >= total_ms) return;
+            const step: u64 = @min(backoff_poll_slice_ms, total_ms - waited);
+            std.Io.sleep(
+                self.io,
+                std.Io.Duration.fromNanoseconds(@intCast(step * std.time.ns_per_ms)),
+                .awake,
+            ) catch |e| switch (e) {
+                error.Canceled => return error.Canceled,
+            };
+            waited += step;
+        }
     }
 
     fn doGetConditionalWithRetry(
@@ -2619,3 +2633,81 @@ test "CountingWriter.sendFile: clamps the source so a file cannot overshoot the
     try std.testing.expect(cw.limit_exceeded);
     try std.testing.expectEqual(@as(usize, 10), inner.writer.end);
 }
+
+// Predicates for the retry-backoff cancellation tests. `TripsLater` proves the
+// backoff keeps polling while it waits: a cancel that arrives after the sleep
+// has already started is the real Ctrl-C case, and an up-front-only check
+// would sleep straight through it.
+const AlwaysCancel = struct {
+    fn cancel() bool {
+        return true;
+    }
+};
+const NeverCancel = struct {
+    fn cancel() bool {
+        return false;
+    }
+};
+const TripsLater = struct {
+    var calls: usize = 0;
+    fn reset() void {
+        calls = 0;
+    }
+    fn cancel() bool {
+        calls += 1;
+        return calls > 1;
+    }
+};
+
+test "retrySleep reports a cancel that is already pending" {
+    var threaded: std.Io.Threaded = .init(std.testing.allocator, .{});
+    defer threaded.deinit();
+    var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = threaded.io() };
+    var http = HttpClient.initWith(&inner, threaded.io(), std.process.Environ.empty, std.testing.allocator);
+    defer http.deinit();
+    // A long budget: without the predicate poll this waits it out and reports
+    // success, which is the bug.
+    http.retry_backoff_ms = &.{4000};
+    http.cancel = &AlwaysCancel.cancel;
+
+    try std.testing.expectError(error.Canceled, http.retrySleep(0));
+}
+
+test "retrySleep observes a cancel that arrives mid-backoff" {
+    var threaded: std.Io.Threaded = .init(std.testing.allocator, .{});
+    defer threaded.deinit();
+    var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = threaded.io() };
+    var http = HttpClient.initWith(&inner, threaded.io(), std.process.Environ.empty, std.testing.allocator);
+    defer http.deinit();
+    http.retry_backoff_ms = &.{200};
+    TripsLater.reset();
+    http.cancel = &TripsLater.cancel;
+
+    try std.testing.expectError(error.Canceled, http.retrySleep(0));
+    // More than one poll: the wait is sliced, not a single up-front check.
+    try std.testing.expect(TripsLater.calls > 1);
+}
+
+test "retrySleep waits out the backoff when nothing cancels" {
+    var threaded: std.Io.Threaded = .init(std.testing.allocator, .{});
+    defer threaded.deinit();
+    var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = threaded.io() };
+    var http = HttpClient.initWith(&inner, threaded.io(), std.process.Environ.empty, std.testing.allocator);
+    defer http.deinit();
+    http.retry_backoff_ms = &.{10};
+    http.cancel = &NeverCancel.cancel;
+
+    try http.retrySleep(0);
+}
+
+test "retrySleep completes when no cancel predicate is wired" {
+    var threaded: std.Io.Threaded = .init(std.testing.allocator, .{});
+    defer threaded.deinit();
+    var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = threaded.io() };
+    var http = HttpClient.initWith(&inner, threaded.io(), std.process.Environ.empty, std.testing.allocator);
+    defer http.deinit();
+    http.retry_backoff_ms = &.{10};
+    http.cancel = null;
+
+    try http.retrySleep(0);
+}

tests/net_get_to_writer_test.zig

@@ -20,6 +20,9 @@ const Mode = enum {
     ok,
     /// First GET answers 503 (a transient error net retries), later ones 200.
     err_then_ok,
+    /// Every GET answers 503, so the loop retries until its budget or a
+    /// cancel stops it.
+    always_err,
     /// First GET answers 401 (a non-transient status the caller re-auths on),
     /// later ones 200 — mirrors GHCR's re-auth handshake.
     unauth_then_ok,
@@ -31,6 +34,15 @@ const Mode = enum {
     redirect_then_ok,
 };
 
+// Set by the always_err stub once it has answered an attempt; the cancel
+// predicate below reads it so the cancel lands on the retry, not before the
+// first request is even sent.
+var served_one: std.atomic.Value(bool) = .init(false);
+
+fn cancelAfterFirstServe() bool {
+    return served_one.load(.acquire);
+}
+
 const Stub = struct {
     io: std.Io,
     listener: *net.Server,
@@ -62,6 +74,12 @@ fn serve(s: *Stub) void {
                     .status = .found,
                     .extra_headers = &.{.{ .name = "location", .value = "/final" }},
                 }) catch return,
+            .always_err => {
+                // Signal that an attempt has been served, so the test's cancel
+                // predicate only trips once a retry is actually pending.
+                served_one.store(true, .release);
+                req.respond("service unavailable\n", .{ .status = .service_unavailable }) catch return;
+            },
             .err_then_ok => if (s.get_count == 1)
                 req.respond("service unavailable\n", .{ .status = .service_unavailable }) catch return
             else
@@ -372,3 +390,44 @@ test "getToWriter trips ResponseTooLarge when the 200 body exceeds the blob cap"
     try std.testing.expectError(error.ResponseTooLarge, status);
     try std.testing.expect(sink.writer.buffered().len <= 8);
 }
+
+test "getToWriter stops re-dialling when a cancel lands on the retry backoff" {
+    // The backoff sits between a transient failure and the next attempt. Until
+    // it honoured the cancel predicate, a Ctrl-C there was slept off and the
+    // request re-dialled through the whole budget. One GET, not four, is the
+    // user-visible contract; the sliced polling itself is pinned by the unit
+    // tests next to `retrySleep`.
+    var threaded: std.Io.Threaded = .init(std.testing.allocator, .{});
+    defer threaded.deinit();
+    const io = threaded.io();
+
+    var addr = try net.IpAddress.parseIp4("127.0.0.1", 0);
+    var listener = try addr.listen(io, .{ .reuse_address = true });
+    defer listener.deinit(io);
+    const port = listener.socket.address.getPort();
+
+    served_one.store(false, .release);
+    var stub = Stub{ .io = io, .listener = &listener, .mode = .always_err };
+    const server_thread = try std.Thread.spawn(.{}, serve, .{&stub});
+
+    var url_buf: [64]u8 = undefined;
+    const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/blobs/sha256:abc", .{port});
+
+    var inner: std.http.Client = .{ .allocator = std.testing.allocator, .io = io };
+    var http = client.HttpClient.initWith(&inner, io, std.process.Environ.empty, std.testing.allocator);
+    // A budget long enough that sleeping it off would be unmistakable.
+    http.retry_backoff_ms = &.{ 4000, 4000, 4000 };
+    http.cancel = &cancelAfterFirstServe;
+
+    var sink: std.Io.Writer.Allocating = .init(std.testing.allocator);
+    defer sink.deinit();
+
+    const status = http.getToWriter(url, &.{}, &sink.writer, null);
+
+    http.deinit();
+    server_thread.join();
+
+    try std.testing.expectError(error.Canceled, status);
+    // The retry never happened: the cancel ended the wait instead of it.
+    try std.testing.expectEqual(@as(usize, 1), stub.get_count);
+}