Fix/unbounded tosend buffer

[John Walicki (johnwalicki)]

Fix unbounded ToSend buffer that crashed the module out of memory

Fixes the viam_filtered-camera crash on bijan-test-windows-2 (module 2.1.0, windows/amd64). The module exited with exit_code=2 after its send buffer grew to 1024 entries, then restarted and immediately began growing again at 1/sec.

Root cause

Two independent defects had to line up:

1. A zero timestamp read as "inside the capture window." Before any trigger fires, captureFrom and captureTill are the zero time.Time. A source camera that returns an unpopulated ResponseMetadata gives us a zero CapturedAt. The window check ended in || now.Equal(ib.captureFrom), so zero.Equal(zero) was true and every background frame was filed as "inside a capture window" that had never been opened.
2. toSend had no cap. ringBuffer is bounded by maxImages; toSend was only drained when data management asked for images, and past the warning threshold it just logged. Nothing stopped it growing.

The log confirms the sequence. After the restart at 11:26:15.97, the first warning at 11:28:16.98 reports size 121 — 121.01 seconds later, i.e. buffer size == seconds since startup, to the frame, with zero drainage. And only image_buffer.go:348 (StoreImages) ever warns; MarkShouldSend's identical warning at :128 never appears, so no trigger ever fired and the window was never legitimately open.

The goroutine dump carries fp=/sp=/pc= on every frame and includes runtime-internal frames — a GOTRACEBACK=system dump, which the Go runtime forces only for a runtime throw, not an ordinary panic.

What changed

• rdk v0.124.0-rc0 → v1.1.0 (and go.viam.com/utils → v0.8.1), via the make update recipe. No source changes were required.
• Zero timestamps no longer count as inside the window. Both copies of the window predicate now route through one withinCaptureWindowLocked, which rejects an uninitialized window (captureTill.IsZero()) and an unstamped frame (now.IsZero()) before the inclusive Equal comparisons run. Unstamped frames land in the bounded ring buffer instead.
• toSend is now hard-capped at max(maxImages * 5, 100) — 300 for the config that crashed, above the existing warning threshold of 120 so the warning still fires first. Enforced on both append paths (StoreImages and MarkShouldSend), dropping oldest and copying into a fresh backing array so the shed images are actually collectable rather than pinned by a re-slice.
• Missing CapturedAt is filled in with wall-clock time by normalizeCapturedAt, applied in captureImageInBackground and in the data-management path of images(). The latter matters because MarkShouldSend(meta.CapturedAt) would otherwise anchor the whole capture window to year 1. Warns once, naming the offending camera, rather than once per frame.
• Buffer notices are edge-triggered. Capping toSend means it now sits pinned at the cap under a slow consumer, so the old per-frame warning would have flooded the log forever (it was already 900+ lines in the crash log). Each episode now logs one WARN at the threshold and one ERROR at the cap, re-arming once the buffer recovers.
• Removed the unreachable single-image path. camera.Camera has no Image method in rdk v1.1.0, neither model defines one, and both images() callers pass false, so singleImageMode could never be true. Deleted PopFirstToSend (whose toSend[1:] re-slice retained its backing array) and the parameter threaded through getBufferedImages()/images() in both models.

No config attributes changed; existing configs need no edits.

Commits

Commit	Subject
e8ed509	Update rdk to v1.1.0
da211bb	Do not treat a zero timestamp as inside the capture window
08a1b6e	Bound the ToSend buffer so a lagging consumer cannot exhaust memory
f40d0c0	Substitute wall-clock time when the source camera omits CapturedAt
9288c84	Remove the unreachable single-image code path

Commits 2 and 3 are the two independent halves of the crash — either alone would have prevented it. Commit 4 fixes the upstream input that triggered it. The ordering is deliberate: containment lands first, so the tree is never in a state where the OOM is reachable.

Files

• image_buffer/image_buffer.go — window predicate, toSend cap, edge-triggered notices, PopFirstToSend removed
• cam.go — normalizeCapturedAt, singleImageMode removed
• conditional_camera/conditional_cam.go — singleImageMode removed (mechanical; forced by deleting PopFirstToSend)
• image_buffer/image_buffer_test.go, cam_test.go — 5 new regression tests
• README.md — documents that both buffers are bounded and what the two log messages mean; drops the Image() bullet for the removed path
• go.mod, go.sum — rdk bump

Testing

• 5 new regression tests. TestBackgroundCaptureWithUnstampedCamera replays the actual failure: a camera returning empty ResponseMetadata, 1024 background captures (17 min at 1Hz), asserting toSend stays at 0. Against the old code that test produces a 1024-entry buffer.

Not addressed

conditionalCamera still passes raw meta.CapturedAt to the shared buffer. It cannot reproduce this OOM — no background worker, and its only write path is the bounded AddToRingBuffer — but with an unstamped source camera it will silently capture nothing. Left for a separate change.

[John Walicki (johnwalicki)]

I did a hot reload of this patched module onto the bijan-temp-windows machine and the errors were resolved.