WTF: add uv_get_cgroup_current_memory()#288
Conversation
Reads the process's cgroup's current memory usage (memory.current on cgroup v2, memory.usage_in_bytes on cgroup v1), reusing the existing uv__slurp, uv__read_uint64 and uv__cgroup1_find_memory_controller helpers that uv_get_constrained_memory() already uses. Matches libuv's uv__get_cgroup_current_memory. Bun needs this for process.availableMemory(): Node.js documents that API as uv_get_available_memory(), which on Linux returns (cgroup limit - cgroup current usage) when a limit is set. The limit reader already exists here; this adds the usage reader so bun can compute the difference without duplicating the cgroup parsing.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughChangesCgroup memory API
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — small, self-contained port of libuv's cgroup current-memory reader that mirrors the existing uv_get_constrained_memory() structure in the same file. The inline note about leaf-vs-hierarchy asymmetry is a follow-up nit, not a blocker.
Extended reasoning...
Overview
Adds uv_get_cgroup_current_memory() to Source/WTF/wtf/uv_get_constrained_memory.cpp plus a header declaration. On Linux it slurps /proc/self/cgroup, dispatches on the 0::/ prefix to cgroup v1 vs v2, and reads memory.usage_in_bytes / memory.current respectively via the existing uv__slurp / uv__read_uint64 / uv__cgroup1_find_memory_controller helpers. A no-op stub is added for non-Linux. ~50 lines, no changes to existing functions.
Security risks
None. Read-only access to procfs/cgroupfs paths that are constructed the same way the adjacent limit reader already constructs them; no user-controlled input, fixed-size stack buffers with snprintf, and the result is an advisory integer.
Level of scrutiny
Low. This is a near-verbatim copy of upstream libuv's uv__get_cgroup*_current_memory and structurally identical to uv_get_constrained_memory() a few lines above (same slurp, same v1/v2 dispatch, same fallback-to-root pattern for v1). The consumer is process.availableMemory(), a best-effort informational API.
Other factors
The one finding is a nit about the v2 reader looking only at the leaf while this fork's limit reader walks ancestors — it can overestimate headroom in multi-container pods with pod-level limits, but it matches upstream libuv's behavior and is a strict improvement over the status quo. It's flagged inline as a possible follow-up, not a correctness bug in this change.
| static uint64_t uv__get_cgroup2_current_memory(char buf[1024]) | ||
| { | ||
| char filename[4097]; | ||
| char* p; | ||
| int n; | ||
|
|
||
| p = buf + strlen("0::/"); | ||
| n = (int)strcspn(p, "\n"); | ||
|
|
||
| snprintf(filename, sizeof(filename), | ||
| "/sys/fs/cgroup/%.*s/memory.current", n, p); | ||
| return uv__read_uint64(filename); | ||
| } |
There was a problem hiding this comment.
🟡 Minor asymmetry with this fork's limit reader: uv__get_cgroup2_memory_limits() walks leaf→root and returns the tightest ancestor memory.max/memory.high, but this reads memory.current only at the leaf. When the effective limit comes from an ancestor (e.g. a K8s pod-level limit with sidecar containers), ancestor.max - leaf.current will overestimate process.availableMemory() by whatever siblings have charged against the ancestor. Fine to land as-is since it matches upstream libuv and is still strictly better than today, but a follow-up that computes min(limit_i - current_i) across the hierarchy (or reads memory.current at the same level whose limit was selected) would make the two consistent.
Extended reasoning...
What the bug is
This fork's uv__get_cgroup2_memory_limits() (Source/WTF/wtf/uv_get_constrained_memory.cpp:186-224) diverges from upstream libuv: it walks from the leaf cgroup up to /sys/fs/cgroup and returns the tightest memory.max/memory.high found at any ancestor. The comment at line 203 is explicit: "cgroup v2 limits are hierarchical: walk from the leaf to the root, taking the tightest limit observed at any level."
The new uv__get_cgroup2_current_memory() copies upstream libuv verbatim and reads memory.current only at the leaf. It does not walk the hierarchy.
The PR description states these two values will be subtracted to implement process.availableMemory() (Node's uv_get_available_memory() semantics: cgroup limit - cgroup current usage). Pairing an ancestor-level limit with a leaf-level usage is not apples-to-apples, because in cgroup v2 an ancestor's memory.current aggregates the usage of all its descendants — including the leaf's siblings — and it is that aggregate that is enforced against the ancestor's memory.max.
Step-by-step example
Kubernetes pod with a pod-level memory limit and two containers (app + sidecar):
/sys/fs/cgroup/kubepods/pod-X/ memory.max = 1Gi memory.current = 900Mi
/sys/fs/cgroup/kubepods/pod-X/container-A memory.max = max memory.current = 450Mi ← this process
/sys/fs/cgroup/kubepods/pod-X/container-B memory.max = max memory.current = 450Mi
uv_get_constrained_memory()walks the hierarchy: leafcontainer-Ahasmemory.max = max, so it climbs topod-Xand returns 1Gi. ✅ Correct.uv_get_cgroup_current_memory()reads only leafcontainer-A/memory.currentand returns 450Mi.- Bun computes
availableMemory = 1Gi - 450Mi ≈ 574Mi. - Reality: the pod's
memory.currentis already 900Mi, so only ~100Mi is allocatable before the pod is OOM-killed. The result overestimates headroom by ~5.7×.
Why existing code doesn't prevent it
Upstream libuv is internally consistent because it reads both the limit and the current usage at the leaf only — so even if it misses an ancestor limit, the two numbers are at least from the same accounting scope. This fork enhanced the limit reader to be hierarchy-aware but the new usage reader is leaf-only, creating the asymmetry. Nothing on the Bun side can reconcile the two scalars after the fact, because the caller doesn't know which ancestor supplied the limit.
Impact
process.availableMemory() is advisory/best-effort, so this won't crash anything, and the single-container-per-pod case (very common) is unaffected because the leaf and the constraining ancestor coincide. But multi-container pods with a pod-level (rather than per-container) memory limit are a normal K8s deployment pattern (sidecars, service meshes), and there the reported headroom will be inflated by the siblings' usage — which somewhat undermines the PR's stated goal.
How to fix
The cleanest fix is a single hierarchy walk that computes min(memory.max_i - memory.current_i) across levels and exposes that directly as "available", since the two independent scalars can't be correctly combined by the caller. A lighter fix is to have uv__get_cgroup2_current_memory() walk the same hierarchy and return memory.current from the level whose memory.max was tightest (though that requires the limit reader to communicate which level that was). Either is reasonable as a follow-up; this PR is still a strict improvement over the status quo.
|
@robobun fix the arm64 build failure |
Preview Builds
|
2 participants
Reads the process's cgroup's current memory usage (
memory.currenton cgroup v2,memory.usage_in_byteson cgroup v1), reusing the existinguv__slurp,uv__read_uint64anduv__cgroup1_find_memory_controllerhelpers thatuv_get_constrained_memory()already uses. Matches libuv'suv__get_cgroup_current_memory.Bun needs this for
process.availableMemory(): Node.js documents that API asuv_get_available_memory(), which on Linux returns(cgroup limit - cgroup current usage)when a limit is set. The limit reader already exists here; this adds the usage reader so bun can compute the difference without duplicating the cgroup parsing.Consumer: oven-sh/bun#34094