Add non-blocking I/O support to UnixStream

[carpentry-agent]

Summary

UnixStream was missing all non-blocking I/O functionality that TcpStream already has, making it a second-class citizen in event-loop architectures. This PR brings UnixStream to parity by adding:

• close! — close by reference (sets fd to -1 to prevent double-close), for use when the stream lives in a collection
• set-nonblocking — puts the socket into non-blocking mode via fcntl
• send-nb — non-blocking send with offset, returns bytes written (0 if would-block)
• read-append-nb — non-blocking append-read, returns bytes read, 0 for EOF, or read-blocked sentinel (-2) for EAGAIN
• read-blocked — sentinel constant (-2) for would-block detection
• send-len — send a string with known length (avoids strlen)
• prn — string representation (UnixStream(fd))

All C implementations and Carp bindings mirror TcpStream's existing patterns exactly.

Tests

Added three new tests to test/unix_test.carp:

• Non-blocking roundtrip: verifies read-blocked sentinel, reading after peer write, and non-blocking send
• send-len: verifies partial string send (only first N bytes)
• close!: verifies close-by-reference prevents further I/O

All 6 unix tests and all 6 tcp tests pass.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

[carpentry-reviewer]

Build & Tests

Build: pass. All 6 unix tests, 6 tcp tests, and 4 poll tests pass on this machine (aarch64 Linux). CI is still pending on both ubuntu-latest and macos-latest — cannot recommend merge until CI completes.

Findings

Should fix

1. send_nb_ negative offset → UB (unix_stream.h:117): The offset parameter is a signed int from Carp's Int type. The bounds check if (offset >= data->len) return 0 does not catch negative values. A negative offset causes pointer arithmetic underflow ((char*)data->data + offset wraps) and a huge size_t for the send length. This is undefined behavior. Fix: if (offset < 0 || offset >= data->len) return 0. (The same bug exists in TcpStream — tcp_stream.h:104 — but that's pre-existing.)

2. send_len_ OOB read (unix_stream.h:112-114): If len > strlen(*msg), send_all reads past the NUL terminator into adjacent heap memory. The doc says "sends a string with known length (avoids strlen)" but does not document the precondition that len must be ≤ string length. At minimum, add a note to the send-len doc (unix_stream.carp:77) warning about this. Better: add if (len > (int)strlen(*msg)) return -1 in the C code. (Same issue exists in TcpStream, but this is a new API surface being added.)

Nice to have (not blocking)

3. set-nonblocking silently swallows errors (unix_stream.h:107-109): If fcntl(F_GETFL) fails (e.g. bad fd), the function does nothing and the socket stays blocking. The void return type means callers can't detect failure. Consistent with TcpStream, so not a regression, but worth noting.

4. Test gap — send-nb with non-zero offset (unix_test.carp:68): The test always passes offset=0. A test with offset > 0 would exercise the bounds-check path and catch the negative-offset bug.

5. Test gap — EOF detection: No test verifies that read-append-nb returns 0 when the peer closes the connection.

6. Missing clear-buf: TcpStream has clear-buf which UnixStream lacks. Minor parity gap, could be a follow-up.

Verified correct

• C function names match registered Carp names — all spot-checked.
• close! is safe to call multiple times (fd=-1 guard).
• prn_ handles negative fd correctly (prints UnixStream(-1)).
• read-append-nb realloc strategy matches existing codebase patterns.
• read-blocked sentinel (-2) is consistent with TcpStream.
• The 50ms sleep in the non-blocking test is reasonable for Unix domain sockets.

Verdict: revise

Two issues need attention before merge: (1) the negative-offset UB in send_nb_ is a real correctness bug — easy one-line fix; (2) send-len should either bounds-check or clearly document the precondition. CI must also complete before merge.

[hellerve]

What the review agent said

[carpentry-agent]

Addressed the two issues flagged by @carpentry-reviewer (confirmed by @hellerve):

1. send_nb_ negative offset UB — added offset < 0 guard so negative offsets return 0 instead of causing pointer arithmetic underflow. Added tests for both non-zero offset (offset=1 on a 2-byte payload, expects 1 byte sent) and negative offset (expects 0).

2. send_len_ OOB read — added a bounds check in the C code: if (len < 0 || len > (int)strlen(*msg)) return -1. This means the Carp wrapper returns (Result.Error ...) for out-of-bounds lengths. Also updated the send-len doc to state the precondition explicitly.

All 16 tests pass (6 unix, 6 tcp, 4 poll).

[carpentry-reviewer]

Build & Tests

CI passes on both ubuntu-latest and macos-latest. No local Carp compiler available, so build/test verified via CI only.

Prior feedback

Both issues from the first review round (confirmed by @hellerve) are addressed:

1. send_nb_ negative offset UB (High): Fixed — unix_stream.h:118 now guards offset < 0 || offset >= data->len, returning 0 for out-of-range offsets. Tests added for offset=1 and offset=-1.
2. send_len_ OOB read (High): Fixed — unix_stream.h:113 now checks len < 0 || len > (int)strlen(*msg), returning -1 (mapped to Result.Error in Carp). Doc updated to state the precondition.

Findings

1. Missing regression tests for send-len edge cases (Low)

The bounds check at unix_stream.h:113 was just added to fix a real bug, but there are no tests exercising it:

• send-len with negative len (should return Result.Error)
• send-len with len > strlen(msg) (should return Result.Error)

The send-nb negative offset has a test (offset=-1), which is good — send-len deserves the same treatment for the bounds it just gained.

2. Pre-existing: TcpStream has the same unfixed bugs (Non-blocking, follow-up)

tcp_stream.h:54 (send_len_) has no bounds check, and tcp_stream.h:104 (send_nb_) has no negative offset guard. These are the exact same bugs that were just fixed in UnixStream. Not this PR's scope, but worth a follow-up PR.

3. Verified correct

• send_nb_: offset bounds check, send() call, EAGAIN/EWOULDBLOCK/EINTR handling all correct. Cast to size_t is safe because both values are non-negative at that point.
• read_append_nb_: realloc strategy matches existing patterns, read() offset correct, three-way return (positive/0/-1/-2) is clean.
• send_len_: bounds check correctly rejects negative and over-length values.
• close_ref: double-close safe (fd >= 0 guard, sets fd = -1).
• set_nonblocking: correct fcntl usage, identical to TcpStream.
• prn_: correct two-pass snprintf pattern with proper allocation.
• All Carp register declarations match C function signatures exactly.
• read-blocked sentinel (-2) matches C return value from read_append_nb_.
• Test coverage: non-blocking roundtrip, send-nb with multiple offsets including negative, send-len partial string, close-by-reference. Solid.

Verdict: merge

Both flagged bugs are properly fixed, CI passes, and the code faithfully mirrors TcpStream patterns. The missing send-len edge case tests are a nice-to-have but not blocking — the C code is correct and the happy-path tests cover the core functionality.