Cap FaultStore, reap abandoned OPC-UA sessions, add heapUsedBytes to /api/system#20
Merged
Conversation
…edBytes Closes the two real unbounded-growth paths found in the memory-leak investigation (the steady-state RSS creep itself was confirmed to be allocator page retention, not a leak -- heap identical under `leaks` across 10 minutes of load, RSS plateaued flat), and adds the allocator-level metric that makes that distinction visible without external tooling: - FaultStore: retention cap of 200 (newest kept), enforced in memory AND on disk -- record() evicts the oldest entry + its report files, and an over-full crash dir from prior runs is trimmed at boot. Previously every fault's full JSON (stack traces included) was retained forever, and scanDir() reloaded the ever-growing dir each boot; a fault-looping module leaked without bound. - OPC-UA sessions: a half-open pushchannel (client vanished without a FIN -- laptop sleep, cable pull) kept pushConns non-empty forever, pinning the session and every monitored item's lastJson. Sessions with connections but no keep-alive past 3x their timeout are now force-closed and reaped. All known clients keep-alive well inside 1x (LoomUI 10s, lux-connect 20s vs the 30s default). Verified: abandoned session reaped at exactly 3x with the socket closed as "session expired"; a keep-alive session is untouched. - SystemMetrics: new heapUsedBytes (malloc_zone_statistics on macOS, mallinfo2 on glibc >= 2.33, 0 = unavailable elsewhere) on /api/system (current + history) and the WS system block. This is the leak-vs-churn discriminator RSS can't provide: RSS climbing with flat heapUsed is page retention; both climbing together is a real leak. Tests: 116/117 (same pre-existing TraceCacheTest failure). Fault cap verified with a 250-file crash dir (200 survive, newest-first, disk pruned).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is a runtime diagnostics + robustness follow-up to the memory investigation: it adds an allocator-reported live-heap metric to the system telemetry, caps persisted fault reports to prevent unbounded growth across runs, and reaps OPC-UA REST pushchannel sessions that remain connected but silent past a grace window.
Changes:
- Add
heapUsedBytesto/api/system(current + history) and to the WSsystempayload, sourced from platform allocator introspection where available. - Cap
FaultStoreto the newest 200 entries, trimming over-full crash directories at boot and evicting oldest entries (and their disk files) on record. - Reap OPC-UA sessions that have pushchannel connections but no keep-alive activity for
3× timeout, force-closing associated connections before removing the session.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| runtime/src/server.cpp | Adds heapUsedBytes to the WS system JSON block. |
| runtime/src/opcua_rest_server.cpp | Adds session reaping logic for connected-but-silent pushchannels past 3× timeout. |
| runtime/src/diag/system_metrics.cpp | Implements readHeapUsedBytes() and records/logs heap usage in the sampler loop. |
| runtime/src/diag/fault_store.cpp | Introduces boot-time trimming and per-record eviction to enforce the fault retention cap. |
| runtime/src/api/router.cpp | Extends /api/system JSON (current + history) to include heapUsedBytes. |
| runtime/include/loom/diag/system_metrics.h | Extends SystemSample with heapUsedBytes and documents the new reader. |
| runtime/include/loom/diag/fault_store.h | Documents and defines kMaxEntries and the new cap enforcement helper. |
Comment on lines
+111
to
119
| void FaultStore::enforceCap() { | ||
| while (entries_.size() > kMaxEntries) { | ||
| const auto& oldest = entries_.front(); | ||
| std::error_code ec; | ||
| std::filesystem::remove(crashDir_ / (oldest.summary.id + ".json"), ec); | ||
| std::filesystem::remove(crashDir_ / (oldest.summary.id + ".txt"), ec); | ||
| entries_.erase(entries_.begin()); | ||
| } | ||
| } |
Comment on lines
+36
to
+40
| /// Retention cap: newest kMaxEntries faults are kept, in memory AND on | ||
| /// disk (older report files are deleted as new ones arrive). Without a cap | ||
| /// a fault-looping module grows the store without bound — each entry holds | ||
| /// the full report JSON including stack traces — and the crash dir | ||
| /// compounds across runs (scanDir() reloads every file at boot). |
2 tasks
…review) - enforceCap() erased entries_.begin() one at a time -- O(n^2) shifting when boot finds a crash dir left thousands of reports deep by a fault loop. Now deletes the excess files in one pass and erases the front range in a single call. - New DiagFaultStore.RetentionCapPrunesMemoryAndDisk test: seeds cap+50 on-disk reports, asserts construction keeps exactly the newest kMaxEntries (memory and disk, newest-first ordering), and that record() at the cap evicts precisely the oldest entry and its file.
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
Follow-up to the memory-leak investigation prompted by the new
/api/systemreadout. The observed steady RSS creep was confirmed not to be a leak (heap byte-identical underleaksacross 10 minutes of workbench-profile load; RSS plateaued completely flat at 10.19MB) — but the code audit found two real, conditional unbounded-growth paths, both fixed here, plus the metric that makes this distinction observable in-process:record()evicts oldest entries and their disk files, and boot trims an over-full dir.heapUsedByteson/api/system(current + history) and the WSsystemblock — live-allocation bytes from the allocator's own bookkeeping (malloc_zone_statistics/mallinfo2; 0 = unavailable). RSS climbing with flat heapUsed = allocator page retention; both climbing = real leak.Test plan
TraceCacheTestone)./api/faultsand prunes disk to 200.session expired, re-attach rejected; a concurrently kept-alive session survived untouched.heapUsedByteslive on a booted runtime: 554KB live heap vs 9.0MB RSS — exactly the retention gap the investigation measured externally.🤖 Generated with Claude Code