Repository navigation
Conversation
…bugs Add C, Rust and Go tests on POSIX and Windows that reproduce externally reported and code-review robustness bugs: - SHM sessions not released after the client leaves (Rust/Go POSIX; all languages on Windows for crashed clients) - a silent client blocking accepts and server shutdown - unbounded client connect/handshake - Rust UDS/named-pipe calls failing after one 100 ms poll slice - shutdown blocked by a worker writing to a non-reading client - stop issued before run being lost - SHM clients waiting the full timeout after a server crash - UDS messages larger than SO_SNDBUF failing with the default packet size Tests for still-open bugs are skipped unless NETIPC_REPRO_KNOWN_BUGS=1; guards for already-correct behavior always run. Wire test_win_robustness into the Windows CI job and MSYS validation script, close SOW-0040, and track the fixes in SOW-0041 to SOW-0045.
Rust UDS and named-pipe receive returned Timeout on the first empty 100 ms abort-poll slice, so any call whose handler took longer than about 100 ms failed regardless of the configured call timeout. An empty slice now means "wait again", as in C and Go; Timeout is returned only at the real deadline, and each wait lasts at least 1 ms. Remove the NETIPC_REPRO_KNOWN_BUGS gate: robustness reproduction tests always run under a zero-known-bug policy, recorded in AGENTS.md. Add Rust transport tests for delayed responses with an abort flag, and run every cross-language service interop pair a second time with a 300 ms server handler on POSIX and Windows. Record new analysis findings in SOW-0042 to SOW-0045 and open SOW-0046 (Go linux/arm64 build) and SOW-0047 (abort signal spec conformance).
The transport specs required an abort descriptor or event waited on together with the connection, but Rust and Go poll an abort flag every 100 ms on all transports and C does the same for SHM waits. Document that a call blocked waiting for its response observes an abort within 100 ms on every transport, and that an abort-check slice is never the call deadline.
Linux rejects a SEQPACKET datagram larger than SO_SNDBUF minus 32 bytes, but C, Rust and Go proposed the full SO_SNDBUF as packet size, so every message that needed chunking failed with EMSGSIZE. Each side now proposes min(SO_SNDBUF - 32, 256 KiB), clamps a configured packet size to the same limit, and clients reject a HELLO_ACK whose agreed packet size exceeds their proposal. Add tests for the kernel limit, clamping and agreed-size rejection in all three languages, and document the derivation in the UDS spec. Widen SOW-0046 to the Go POSIX build failures found on linux/386, arm, arm64, riscv64, freebsd and darwin.
Go POSIX transport now builds on every Linux architecture: - poll through a shared internal/syspoll package (ppoll on Linux, which is the only option on arm64, riscv64 and loong64) - Msghdr.Iovlen set from untyped constants (width differs per arch) - sendmsg via socketcall(2) on linux/386, direct sendmsg elsewhere - 32-bit-safe test constants and 8-byte aligned test SHM buffers Running the suite on 32-bit exposed SHM bounds bugs, fixed here: - Go ShmReceive compared int(mlen), which wraps negative on 32-bit, so a forged length passed the bound and panicked the receiver - Go, C and Rust client attach summed region ends in 32-bit arithmetic, so a forged header could pass the file-size check on 32-bit targets Runtime Safety CI gains a Linux architectures job: Go vet on nine architectures, Go tests on 386 and under QEMU on arm64 and arm, and a Rust library check for aarch64 and armv7. FreeBSD and macOS support are tracked in SOW-0048 and SOW-0049.
…FS stale cleanup and musl msghdr
SHM servers now end a session when its UDS socket or named pipe reports the client gone, and SHM clients fail a waiting call with a new disconnected error (C NIPC_ERR_DISCONNECTED, Rust NipcError::Disconnected, Go protocol.ErrDisconnected) within one 100 ms slice of the server's death instead of waiting the full call timeout. Peer-gone send/recv failures on UDS, named pipes and SHM report the same error. SHM attach stops retrying once the server closed the session socket. Every socket and SHM descriptor is close-on-exec. The unused owner_alive check is removed. The Windows interop scripts and the Windows benchmark runner now force-stop servers built against the MSYS runtime, which ignore SIGTERM while blocked in a Win32 wait, and resolve taskkill through SYSTEMROOT. Adds the test cadence rules to AGENTS.md and the performance assessment step to the vendoring preflight; SOW-0050 tracks vendoring into Netdata.
Server side, in C (POSIX and Windows), Rust and Go: - Split accept into a pending accept and a handshake with a 5000 ms deadline; the handshake runs on the session worker, so a silent client no longer blocks other accepts or shutdown. - Connections take a worker slot from accept; when all workers are busy the accept loop waits up to 200 ms for one, then refuses the connection before HELLO_ACK. - Stop is a never-cleared stop-requested flag honoured by run(); stop paths only signal and wake, and run owns the listener close. - Shutdown gives workers a 1000 ms grace, then unblocks them with shutdown(SHUT_RDWR) on POSIX or repeated CancelSynchronousIo on Windows. - Windows session close closes the handle only, without FlushFileBuffers, which blocked on clients that stopped reading. Also fixes a double handle close in Go Windows pipe tests, applies the handshake deadline to Go one-step accepts, keeps fuzz, stress and scale suites out of the Linux target runs, and adds handshake-deadline tests in every language. Docs and the integrator skill describe the new guarantees. Client-side deadlines are tracked in SOW-0051, transport hygiene in SOW-0052 and the deterministic test suite in SOW-0053.
Correctness tests in C, Rust and Go now synchronize on events instead of sleeps, polling and retry loops, and shorten production waits through per-instance test timings. Fuzz, stress, randomized chaos, lookup-scale and benchmark-policy suites run only from tests/run-extended-fuzz.sh and the new tests/run-extended-stress.sh. - Rust and Go managed servers expose a listening signal; C init already binds. - Windows clients wait once (at most 1000 ms) on ERROR_PIPE_BUSY instead of failing at once, in C, Rust and Go. - C test threads on MSYS2 start through pthread_create; CreateThread threads could die silently under load. - Interop scripts share tests/interop-lib.sh: block on READY, kill only a process whose identity still matches (including under QEMU binfmt). - Remove the C-only reconnect drain sleep. - Track MSYS thread creation in the library as SOW-0054.
One deadline now covers a whole L2 call or refresh in C, Rust and Go: connect, HELLO_ACK, the Windows busy-pipe wait, SHM attach and every reconnect, and an abort ends any of these waits. New bounded L1 connects (nipc_uds_connect_timeout, nipc_np_connect_timeout, connect_with_timeout, ConnectTimeout) back this; the old connects wrap them without a deadline. C reconnects once per failed call like Rust and Go, and Rust and Go report a call on a non-ready client as NotReady instead of BadLayout. Windows servers no longer call DisconnectNamedPipe before closing a rejected pipe, which discarded the rejection reply before the client read it.
The MSYS runtime does not support threads started with CreateThread; under load such a thread may never run and the process dies silently. Under __MSYS__ the library's session threads and the Windows C benchmark driver's threads now start with pthread_create, and each thread hands the creator a duplicate of its own handle, so CancelSynchronousIo, WaitForSingleObject and CloseHandle keep working on a real thread handle. Native Windows builds keep _beginthreadex. A new test checks that library threads are runtime threads.
…nctly - Retry send, sendmsg and recv on EINTR in the C and Rust UDS transports. - Take an exclusive per-service flock before stale-socket recovery in C, Rust and Go, so a starting server can no longer unlink a live endpoint between bind and listen; drop Go's timed re-dials. - Add a transport error (C NIPC_ERR_TRANSPORT, Rust NipcError::Transport, Go protocol.ErrTransport) for I/O failures with a live peer, map SHM receive overflow to overflow, and align Go's baseline receive limit. - Split SHM region timing into SOW-0055.
Managed servers in C, Rust and Go, on POSIX and Windows, created a per-session SHM region for every accepted connection before the handshake, including rejected and baseline-only clients. The L1 server handshakes gain an optional prepare callback, called with the selected profile and the agreed payload limits after the auth, selection and limit checks and before HELLO_ACK. On failure the server drops that profile and selects again in the same handshake, rejecting with UNSUPPORTED when none is left. Managed servers create the region in the callback, sized from the agreed limits; Windows creates only the selected profile's objects. New tests in all three languages and both platforms count region creations for baseline, rejected, SHM and failed-creation handshakes. The handshake docs now allocate session_id before SHM preparation, as the implementations do.
- Build C, Rust and Go on FreeBSD with the UDS baseline only: CMake gates SHM by NETIPC_POSIX_SHM (Linux), C and Go keep SHM APIs as "unsupported" stubs off Linux, Rust gates Linux-only code. - The POSIX handshake masks offered and accepted profiles to what the platform provides, so non-Linux peers never negotiate SHM. - Clients on POSIX and Windows reject a HELLO_ACK whose selected profile is not exactly one offered bit. - FreeBSD SEQPACKET accepts the full SO_SNDBUF; packet size accounts for it. - Tests and fixtures are portable (no /proc dependence); Rust and Go robustness suites run on every POSIX system. - Docs and AGENTS.md describe the platform behavior and the FreeBSD validation procedure. SOW-0031 merged; SOW-0056 tracks FreeBSD 15.
C, Rust and Go now call fchmod(fd, 0600) on the unbound socket before bind(); Linux and FreeBSD create the socket file with that mode. The C library no longer changes the process-global umask around bind(), so other threads of an embedding program keep their file modes, and Rust and Go sockets no longer depend on the ambient umask. Tests: a bind() wrapper creates an unrelated file during listener setup and checks it keeps the process umask (C); Rust and Go assert the socket is 0600. Docs describe the socket permission contract.
rustfmt.toml selects style edition 2024, matching the language edition and Netdata's Rust workspace. The diff is formatting-only: whitespace, import order, trailing commas and semicolons after tail return expressions. AGENTS.md and README.md describe the new style setting.
…M and Administrators Pipes, SHM mappings and events take a protected DACL granting only SYSTEM, Administrators and the creating account, and pipes reject remote clients. Clients open with SECURITY_IDENTIFICATION and refuse to send HELLO to a pipe whose owner is not their own account, SYSTEM or Administrators (new untrusted-server error, AUTH_FAILED at Level 2). Same change in C, Rust and Go, with DACL, remote-open, owner-check and impersonation-level tests. Closes SOW-0045.
go 1.27.0 in the src, bench and test-fixture modules; the Codacy, supply-chain and static-analysis workflows install Go 1.27.x. Validated with Go 1.27 on Linux (including the Netdata Linux targets), FreeBSD and Windows. Closes SOW-0058.
transport/framing.rs now holds message send with chunking, receive with reassembly and validation, and the HELLO/HELLO_ACK negotiation; the POSIX and Windows transports supply their I/O and error type, like Go's transport/internal/framing. The Windows receive now rejects total lengths that do not fit the u32 total_message_len, and the receive length sum no longer overflows a 32-bit usize on either transport.
macOS has no AF_UNIX SOCK_SEQPACKET, so C, Rust and Go use a stream socket there. Packets keep the SEQPACKET bytes: each is the 32-byte header plus the bytes it declares. The first header is checked before its body is read, and a receive that stops partway through a message shuts down the read side. Owner-only endpoints come from binding in a private 0700 directory and renaming the socket into place. Linux, FreeBSD and Windows keep their code paths. The stream helpers are unit-tested on Linux, and a macOS CI workflow runs the suites and the interop scripts. Test fixes found on macOS: interop-lib.sh now works on bash 3.2 (it used to false-pass there), socket run directories are short, shared test servers use a large enough backlog, and test_hardening no longer overflows the session table.
A child forked by another thread shares the lock's open file description until it execs, so closing the fd alone left the lock held and a listen right after a close could fail with address-in-use. C, Rust and Go now unlock explicitly on close and on every failed-listen path. Tests in each language hold the lock fd in a child process while the listener closes.
The shared framing layer rewrapped every HELLO receive error as a receive failure, so a Go server reported a client that never sent HELLO as ErrRecv, where C and Rust report a timeout or an abort. The POSIX and Windows server handshakes now keep the wait's error, as the client side already does. Tests cover a deadline and an abort on both platforms.
FreeBSD sessions now negotiate the SHM fast path in C and Rust on every architecture and in Go on amd64 and 386, interoperating across all language pairs. The region layout and protocol are unchanged; only the system calls behind wait, wake and reservation differ: - wait/wake use the shared _umtx_op operations (UMTX_OP_WAIT_UINT with a relative CLOCK_MONOTONIC _umtx_time, UMTX_OP_WAKE); a stale expected value returns at once and is handled as a re-check; - region files are reserved with posix_fallocate. ZFS rejects it with EINVAL, so SHM preparation fails and the session uses baseline, as the spec requires; tmpfs and UFS run directories get SHM. Go gains per-OS wait/wake/allocation files and per-architecture raw posix_fallocate syscalls; Rust SHM gates cover FreeBSD; CMake builds SHM and its tests there. Unlinking a directory fails with EPERM on FreeBSD and macOS, not EISDIR. Stale SHM regions and UDS endpoints occupied by an empty directory are now reclaimed there too in C and Rust (Go already did). Docs: FreeBSD SHM syscalls, ZFS and run-directory guidance, platform tables, integrator skill, AGENTS.md FreeBSD commands. SOW-0062 tracks a benchmark-client deadlock on small socket buffers found by the FreeBSD benchmark run.
A client with more than one request in flight sent each packet with a blocking call. Once unread responses filled the buffers, the server blocked sending and the client blocked sending: a write-write deadlock, seen on FreeBSD for all 9 uds-pipeline-batch benchmark pairs (8 KiB SEQPACKET buffers). Pipelining client sends in C, Rust and Go now use a non-blocking send; while waiting for buffer space they read arriving responses into a per-session queue bounded by the in-flight responses, and receive returns queued data first. macOS, which ignores MSG_DONTWAIT on a stream send short of buffer space, switches the descriptor to O_NONBLOCK for that send. Overflowing the bound fails the session and every in-flight request. One-in-flight calls keep the blocking send. Callers that wait on the session fd check the new has-pending accessor (nipc_uds_session_has_pending, UdsSession::has_pending, Session.HasPending) first. A failed macOS stream receive now drops the queue, and Go's queue bound saturates on 32-bit targets. Docs describe the POSIX guarantee and an interim Windows caller rule; Windows progress is tracked in SOW-0064.
Go benchmark clients read the wall and monotonic clocks five times per request; on FreeBSD guests with a kvmclock timecounter each read is a syscall, so the Go rows measured the harness. The Go POSIX driver now reads a monotonic-only clock twice per request. Go typed servers allocated per request (response header, request views, builders, the cgroups lookup suffix table); the resulting GC cycles stalled the server, so the client slept in futex and every response paid a real wake. Servers now keep the header per session and take request and builder scratch from pools. Typed clients keep the request header and the returned snapshot and string-reverse views in the client, which the documented "valid until the next typed call" lifetime already allows. On Windows the Named Pipe poll and SHM signal/wait calls use SyscallN, since LazyProc.Call allocated on every call. Per-call deadlines use a new monotonic-only clock (internal/monotime), so Go clients read the clock as often as C. STRING_REVERSE decoding now borrows the payload as a []byte, as the codec spec requires, and its handlers take []byte (a test method). Allocation tests cover the typed servers and the increment, snapshot and string-reverse calls over UDS, POSIX SHM, Named Pipe and Windows SHM. Test helpers in C, Rust and Go remove the endpoint lock files they create. Lookup clients still copy response items per call in all three languages; tracked in SOW-0065.
…buffers The client handle is opened overlapped in C, Rust and Go. A stalled client write reads arriving responses into a bounded per-session pending queue, so the server never blocks on a reply the client is not reading. Receive and wait-readable return queued messages first, and a has-pending check is exposed on the Windows session. Go fails ERROR_MORE_DATA receives like C and Rust. The benchmark drivers no longer need a byte budget.
FreeBSD 15 SOCK_SEQPACKET keeps record boundaries only where the sender sets MSG_EOR, lets a non-blocking send write part of a record and delivers large records in parts, so NetIPC packets merged. FreeBSD now uses the stream baseline already used on macOS, in C, Rust and Go, with SHM kept. A stream receive that fails inside a message now marks the session instead of calling shutdown(SHUT_RD), whose effect differs between macOS, FreeBSD 14 and FreeBSD 15: later receives fail, pipelined sends stop queueing inbound bytes, and has-pending reports nothing. Validated on FreeBSD 15.1, FreeBSD 14.3, macOS and Linux.
- Windows DACL tests (Go, Rust) build the expected SDDL through Windows, so accounts with SDDL aliases such as the built-in Administrator match. - The C CLOEXEC test ignores descriptors the process inherited. - armv7 glibc Rust target builds link statically without static-pie. - Extended-only Go test helpers carry the netipc_extended tag; the no-op setNoSigpipe builds only where it is called. - The macOS owner-only bind works relative to a descriptor of its private directory in C, Rust and Go, and survives a umask without owner bits. Go uses golang.org/x/sys v0.49.0, the version Netdata already requires. - Semgrep is pinned; benchmark run directories are created 0700. - Backport the BatchBuilder rustdoc fix from Netdata.
The endpoint lock file and the SHM region file are created with mode 0600, which the umask narrows. Under a umask that strips owner read or write, later opens of those files by the same account fail with EACCES: a second server sees a socket error instead of address-in-use, a lock file left behind blocks every restart, and SHM clients cannot attach. C, Rust and Go now fchmod both files to 0600 through their descriptors. The lock open falls back to write-only, then read-only, when read-write is refused, so a leftover lock file of mode 0200 or 0400 is reopened and repaired. Tests cover umask 0477 and 0277 and leftover lock modes; the Rust and Go umask tests run in a child test process.
The Rust child ran entirely under umask 0477, so the coverage runtime wrote its profile as mode 0200 at exit and llvm-profdata merge crashed in the coverage job. The Rust and Go children now set the umask themselves and restore it before exit, also on failure.
gosec G115 flags uint32(ms) in Client.opRemainingMs; ms is positive and capped at the uint32 maximum on the line before, so the conversion cannot overflow. Use the same #nosec justification form as the other bounded conversions.
MSYS cannot apply POSIX modes on a noacl mount, so mkdir -m 0700 fails and stopped tests/run-windows-bench.sh before its first benchmark. The native Windows drivers do not check the run directory mode; restore plain mkdir -p there. The POSIX script keeps mkdir -m 0700.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Important Review skippedToo many files! This PR contains 370 files, which is 270 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files.
 |
…benchmark hosts SOW-0050 moves to in progress with its preflight, same-host A/B evidence, user decisions and the Netdata vendoring PR; it closes once that PR's CI passes. SOW-0068 tracks restoring Go coverage to its threshold, SOW-0069 the benchmark host drift and floor re-baseline.
Up to standards ✅🟢 Issues
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review completed against the latest diff
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 356 files
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Requires human review: Auto-approval blocked because this review re-detected 26 unresolved issues already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
Bound every send by the call timeout (C, Rust, Go, UDS and Named Pipe), retry accept on EINTR, bound the C POSIX HELLO wait by one handshake deadline, reject malformed HELLO/HELLO_ACK packets on message-oriented transports, repair mode-000 lock files only when owned and regular, reject an empty run_dir, retry busy Windows pipe instances, and pass NULL byte counts to overlapped ReadFile/WriteFile. Add reproduction tests for each deterministic finding, fix the run() exit code in the sanitizer, valgrind, race and fuzz scripts, gate the bench drivers' SHM paths and validate their durations, align the OSV Go version with the release Netdata ships, and update the transport, wire-envelope and typed-API specs, the integrator guide, AGENTS.md and the vendoring skill. Correct records in earlier SOWs.
There was a problem hiding this comment.
All reported issues were addressed across 135 files (changes from recent commits).
Tip: Get a Slack DM when cubic finishes reviewing your PRs. Connect Slack
View guided diff | Turn on auto-fix | Re-trigger cubic
Publish a missing endpoint lock under its name only once it is 0600 (temporary file, fchmod, hard link) in C, Rust and Go, replacing the path-based chmod repair of mode-000 lock files that static analysis flagged. Size the pending queue for one response when a bounded send runs with nothing in flight, which divided by zero. Serialize Go Listener.StopListening so a concurrent Close cannot release the lock before the socket file is gone. Correct bench duration limits, unassigned-profile tests, fixture synchronization and records, and run extended stress on macOS through workflow_dispatch.
| * read-write, so a narrower open follows (flock() works on any access | ||
| * mode) and fchmod() restores 0600 for the next starter. fchmod() fails | ||
| * only for a file another account owns, which the lock still works on. */ | ||
| int fd = open(lock_path, O_RDWR | O_CLOEXEC | O_NOFOLLOW); |
|
|
||
| void nipc_uds_set_listen_hook_for_tests(void (*hook)(void *user), void *user) | ||
| { | ||
| g_listen_hook_user = user; |
| void nipc_uds_set_lock_publish_hook_for_tests( | ||
| void (*hook)(const char *tmp_path, void *user), void *user) | ||
| { | ||
| g_lock_publish_hook_user = user; |
There was a problem hiding this comment.
All reported issues were addressed across 30 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
The Go runtime's preemption signal can interrupt the raw poll in the stalled-reader test helper, and POLLHUP alone does not prove the request arrived. Run only lookup-scale interop in the macOS manual step, since the full extended suite needs SHM, and correct bench comments and usage.
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Robustness, platform and performance work since the last Netdata vendoring (
63919ec, #17), and the source side of the next vendoring. Each change landed as one SOW commit; the SOW files under.agents/sow/done/carry the evidence.Bugs fixed (each with a reproduction test that fails before the fix)
NIPC_ERR_DISCONNECTED, RustNipcError::Disconnected, Goprotocol.ErrDisconnected) within one 100 ms slice instead of the full timeout.msghdr.Security
netipc_win_security.c).Platforms
golang.org/x/sysv0.49.0 (macOS fd-relative bind).Performance
63919ec(what Netdata runs today):benchmarks-*.csvare unchanged: both benchmark hosts now run at about half their June numbers for old and new code alike; tracked by SOW-0069.Tests and CI
tests/run-extended-*.sh.Validation
4261919: see the checks on this PR. Known non-code failures: Codacy coverage upload (token permission) and OpenSSF Scorecard (default branch only).Follow-ups
Netdata vendoring PR: netdata/netdata#24633