feat(ios-profiler): attributable memory leaks via malloc_stack_logging#351
Merged
Conversation
native-profiler-start gains an opt-in `malloc_stack_logging` flag. When set, it cold-launches the target app under xctrace with `--env MallocStackLogging=1` instead of attaching, so Instruments records allocation backtraces and leaks carry a real responsible frame + library. Without it leaks are detected but unattributable — Instruments reports "<Call stack limit reached>". Default behaviour is unchanged: attach to the running app, no relaunch, no overhead. The report now relabels unattributable leaks with a hint to re-run with malloc_stack_logging rather than surfacing the raw placeholder. - split detectRunningApp into reusable AppInfo helpers - resolve the .app bundle path via `simctl get_app_container` for --launch - terminate the running instance first for a clean cold start - tests: launch+env vs attach argv, and the unattributable-leak render
Reference + native-profiler skill now explain the attach-vs-cold-launch trade-off and how to get attributable leaks.
latekvo
force-pushed
the
profiler-attributable-leaks
branch
from
e341e7d to
58a8d62
Compare
…ingUserApps enumerateRunningUserApps inlined the same simctl listapps | plutil | JSON.parse block that the new getInstalledApps helper already provides. Route it through the helper so the two can't drift.
…-leaks # Conflicts: # packages/tool-server/src/utils/ios-profiler/render.ts
latekvo
requested review from
filip131311,
hubgan and
stachbial
and removed request for
filip131311 and
stachbial
hubgan
requested changes
Jun 30, 2026
… FailureError consistency malloc_stack_logging cold-launches the app under `xctrace --device`, which is broken on Xcode 26.4-27.0 (the --device recording-start handshake records an empty trace). The path hardcoded `--device` and never consulted the capture strategy selector, so on those versions it terminated the running app and then captured nothing - surfaced only as a downstream "Analysis failed". - Refuse the malloc path up front (before terminating the app) when the selected capture strategy is not "device", honoring ARGENT_IOS_CAPTURE=device as an override. - Best-effort relaunch the app if the malloc start fails after terminate. - Wrap getAppBundlePath / resolveAppForLaunch plain Errors in FailureError so these reachable failures carry native-profiler error codes for telemetry. - Gate the cold-launch argv on the malloc flag, not launchBundlePath truthiness (removes a latent NPE); reject an empty resolved bundle path. - Fix the leak-attribution test fixture to use a real classifier sentinel. - Document the slow launch and the degraded-Xcode guard. Adds deterministic tests: a degraded Xcode refuses before terminate, and the ARGENT_IOS_CAPTURE=device override forces the cold launch through.
…h fails after terminate
…or a malloc cold start getDebugDir() (which mkdir's the debug dir) ran after the malloc path terminated the running app but before the relaunch-protected start attempt. If that mkdir threw (ENOSPC / EACCES), the app was left killed with no relaunch — the best-effort relaunch only wraps startWithRetry. Hoist the debug-dir/output-path resolution above the branch so any mkdir failure happens before the app is touched. Adds a regression test asserting a getDebugDir failure never terminates the app.
hubgan
reviewed
Jul 1, 2026
hubgan
left a comment
hubgan
left a comment
Collaborator
There was a problem hiding this comment.
A few small notes on the malloc_stack_logging path - low-severity messaging/attribution details and a couple of branches the new tests don't cover. None are blocking; flagging for your consideration.
…killed apps Address review feedback on the malloc_stack_logging path: - Add a side-effect-free resolveIosCaptureStrategy() that returns the chosen strategy plus WHY (env-override vs degraded-xcode vs default). The malloc degraded-Xcode guard now uses it, so it no longer emits selectIosCaptureStrategy's "using the all-processes capture fallback" stderr line immediately before throwing. - When the strategy isn't `device` because the operator forced ARGENT_IOS_CAPTURE=all-processes on a healthy host, refuse with a distinct message and a new NATIVE_PROFILER_MALLOC_STRATEGY_OVERRIDE code instead of blaming a degraded Xcode that isn't present (fixes misleading telemetry). - Only mark the terminated app for best-effort relaunch after the simctl terminate actually succeeds. An installed-but-not-running named app is no longer foregrounded on a failed cold launch (restore only what we killed). - Tests: cover the LAUNCH_APP_NOT_FOUND and MULTIPLE_RUNNING_USER_APPS malloc branches, assert --device <udid> is threaded into the cold-launch argv, assert the override-vs-degraded attribution, and assert the degraded refusal emits no "capture fallback" stderr line. - Docs: note the forced-override refusal in IOS_PROFILER_REFERENCE.md.
Collaborator
|
The feature is dormant on all current Xcode (26.4+), and there's no way to exercise it in CI. isDegraded() (capture-strategy/select.ts:84) flags 26.4+ and all 27+. The malloc path requires --device, so on every current Xcode the guard refuses by default, and forcing it produces an empty trace (E2E-confirmed). In practice the feature only delivers value on Xcode ≤ 26.3. This is inherent to Apple's xctrace regression and the guard is the right mitigation — but reviewers/users should know the feature ships inert on mainstream Xcode, and its core value (attribution) has no automated coverage against a real trace. Consider a tracking note for when Apple fixes --device so the bound can be narrowed. |
Collaborator
|
Unattributed-leak render message is misleading in malloc mode. render.ts:417-421 hardcodes "Argent records via xctrace --attach, which has no malloc-stack history … For attributed stacks, capture with malloc stack logging enabled at launch." When a trace was captured with malloc_stack_logging:true but some leaks still come back unattributed, this tells the user to do the thing they just did, and names the wrong capture mode. Pre-existing (from #337) but newly reachable via this PR — the render layer has no signal of the capture mode. Low impact; worth a follow-up to thread capture-mode into the report. |
Collaborator
|
Residual shell-injection surface in the relocated getInstalledApps. platforms/ios.ts still uses execSync( |
`aggregateLeaks` grouped solely by object type, freezing the responsible frame/library at the first-seen row. That was harmless under `--attach` (every frame is the same `<Call stack limit reached>` sentinel), but this feature's whole point is giving leaks distinct real stacks via malloc_stack_logging — so the common case of one object type leaked from several call sites collapsed into a single row attributed to whichever was parsed first, hiding the other sites. Group by (object type, responsible frame) so each site is its own finding; the sentinel/`--attach` case still collapses to per-type since all frames are equal. Adds a repro test (distinct frames now stay separate; same frame still merges).
hubgan
reviewed
Jul 3, 2026
…g path - Stop the leak group key from making 01-correlate.ts a binary diff: the (objectType, responsibleFrame) delimiter was a raw NUL byte, which flips git to binary mode and hides the PR's core grouping change. Write it as a unicode escape (identical runtime string; file stays text) and pin the collision-safety with a regression test so nobody swaps it for a space. - Warn on an unrecognised ARGENT_IOS_CAPTURE in malloc mode. The guard resolves the strategy via the side-effect-free resolveIosCaptureStrategy(), which never emitted the "ignoring unrecognised ..." warning, so a typo'd override was dropped silently (and the degraded-Xcode refusal could even advise setting the var the user had already fumbled). Extract warnIfInvalidCaptureOverride() and call it from both the normal record flow and the malloc guard. - Harden getInstalledApps to two discrete-argv execFileSync calls (simctl listapps piped into plutil) instead of an execSync shell string, completing the execFileSync conversion the rest of this path already uses. - Make the degraded-Xcode refusal range accurate: isDegraded() blocks 26.4 and all of 27+, so the frozen "(26.4-27.0)" contradicted itself on e.g. 27.5. - Don't tell the user to re-run with malloc stack logging when the capture already attributed some leaks (malloc was clearly on): the unattributed-leak render note now infers capture mode from the attributed count. - Dedup the identical "multiple running user apps" FailureError into one builder.
The execFileSync hardening now captures the full `simctl listapps` plist into Node before piping it to plutil, whereas the old shell pipe only buffered plutil's (smaller) JSON output. The plist is ~1.5-2x larger than the JSON, and neither call set maxBuffer, so a well-populated simulator whose plist exceeds Node's 1 MiB default would throw ENOBUFS where the old code worked. Set a generous 64 MiB maxBuffer on both calls and add a regression test asserting the listapps capture requests a buffer well above 1 MiB.
…eck:tests The maxBuffer regression test read mock.calls[i][2] (the options arg), but the mock fn was typed (bin, args) only, so tsconfig.test.json's stricter typecheck (CI 'Unit tests' job) rejected the missing tuple index. Declare an options param on the mock so the call tuple carries it, and drop the now-needless cast.
hubgan
reviewed
Jul 3, 2026
The only real conflict was the iOS native profiler's running-app enumeration (native-profiler/platforms/ios.ts): this branch and main independently hardened the simctl subprocesses against shell injection, converting execSync shell strings to execFileSync argv. The resolution keeps this branch's getInstalledApps helper refactor but adopts main's hardened details inside it - the 256 MiB DEFAULT_EXEC_MAX_BUFFER on each stage and the `--` separator before plutil's stdin `-` - so main's shell-injection regression test passes unchanged. The 64 MiB LISTAPPS_MAX_BUFFER_BYTES constant it supersedes is dropped, and the now-unused execSync import is removed. main's shell-injection fix also moved launchctl and `xcodebuild -version` from execSync to execFileSync. The auto-merged malloc-stack-logging.test.ts still mocked those through execSync, so its launchctl/xcodebuild stubs were dead against the merged code (running-app detection and the degraded-Xcode guard both saw empty output). Route those mocks through the execFileSync path so the test matches the merged, hardened implementation.
hubgan
reviewed
Jul 7, 2026
hubgan
left a comment
hubgan
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the malloc_stack_logging changes and exercised them end-to-end against an iPhone 16 Pro simulator on Xcode 26.6 and a Pixel 3a emulator. The production logic and its safety guarantees hold up well: on this (degraded) Xcode the malloc path is refused before the running app is terminated - the app's PID was unchanged after the refusal - and forcing ARGENT_IOS_CAPTURE=all-processes yields a refusal that correctly names the override rather than blaming a degraded Xcode. I also confirmed the --device cold launch genuinely deadlocks on 26.6, so the guard is warranted, and on Android the flag is cleanly ignored (no regression). A few inline notes below, mostly around test coverage and docs.
- Correct the degraded-Xcode range in every user/model-facing surface. isDegraded()
treats 26.4 and ALL of 27+ as broken (no upper bound), but the malloc_stack_logging
tool schema, IOS_PROFILER_REFERENCE.md and the capture-strategy docstrings said
"26.4-27.0" — implying 27.1+ works while the call is actually refused up front and
the runtime message already reads "26.4 and later". Reword to match the real bound.
- Mirror render.ts's capture-mode-aware leak hint into renderCombinedMemoryLeaks. In a
mixed report (some leaks attributed, some not) the combined report unconditionally
advised "capture with malloc stack logging enabled at launch" — the exact thing the
user just did, since attributed and unattributed leaks only coexist under malloc mode.
It now names the active malloc capture instead; the combined-report test asserts both
the attach-era and malloc-era hint wording.
- Make the "does NOT relaunch a not-running named app" test actually exercise its
scenario. Its mock returned "" for listapps/plutil, so getInstalledApps hit
JSON.parse("") and threw SyntaxError before simctl terminate ever ran — the
no-op-terminate -> no-relaunch path was never reached (both assertions were satisfied
by that early throw). Feed a valid listapps plist so the path reaches terminate; assert
terminate was attempted and the surfaced error is not a SyntaxError.
- Drop the now-dead execSyncFn xcodebuild/listapps/launchctl branches in the relaunch
and forced-override tests (those subprocesses go through execFileSync now, so the
branches never fired and the "Xcode 16.4" comment misdescribed why the guard passed).
Mock the Xcode version through execFileSync so "non-degraded Xcode -> guard passes" is real.
Member
Author
|
Status on the three top-level notes from 2026-07-02: Unattributed-leak render message misleading in malloc mode — resolved. Residual shell-injection surface in getInstalledApps — resolved. Feature dormant on current Xcode + tracking note — acknowledged; inherent to Apple's |
…ded docstring The isDegraded() docstring still carried "the regression has shipped across every 26.4-27.0 build tested" — the lone surviving bounded-range string after the schema, reference doc and sibling docstrings were switched to "26.4 and later". The surrounding text already says "from 26.4 onwards … ALL of 27+", so it wasn't a contradiction, but a hurried reader could momentarily read "26.4-27.0 build tested" as confining the bug to <=27.0 — the exact misread the fix set out to remove. Reword to "no upper bound … across every build tested so far, 26.4 through 27.x" so the file is uniform.
…alloc mode) getAppBundlePath (simctl get_app_container) runs before the destructive simctl terminate on the malloc cold-launch path — the same "don't leave the app dead" invariant the debug-dir mkdir guard already has a test for, but this sibling ordering was unpinned. A regression that reordered bundle-path resolution after terminate (leaving the user's app killed on an unresolvable container), or that stopped rejecting an empty resolved path (`xctrace --launch -- ""`), would ship green. Add two malloc-mode tests: get_app_container throwing, and returning a blank path. Both assert NATIVE_PROFILER_APP_BUNDLE_PATH_FAILED, that terminate was NOT called, and that xctrace never spawned. Verified they fail when getAppBundlePath is moved below the terminate, and pass with the shipped ordering.
… the malloc refusal The forced-override refusal (mallocNonDeviceStrategyError) interpolated the canonical strategy name into `ARGENT_IOS_CAPTURE="…"`, framed as the env var's literal value. But parseEnvOverride accepts the `all_processes`/`allprocesses` aliases and lower-cases input, so a user who set e.g. `ARGENT_IOS_CAPTURE=all_processes` got a refusal claiming they set `all-processes` — telling them to fix a variable that reads differently from what they typed. The whole point of this refactor round was accurate attribution in these messages. - Thread the literal (trimmed, case-preserved) override value through the env-override CaptureStrategyReason as `rawValue`; the refusal now quotes it for the `ARGENT_IOS_CAPTURE="…"` part while still naming the canonical strategy it resolved to. parseEnvOverride keeps the original for the invalid branch too, so the "ignoring unrecognised …" warning likewise echoes the operator's exact value. - Regression test: set `ARGENT_IOS_CAPTURE=All_Processes` (alias + mixed case) and assert the refusal quotes `All_Processes` verbatim, names the `all-processes` strategy, and does NOT quote the canonical name as the env value. Verified it fails against the pre-fix message and passes after. - Docs: the malloc_stack_logging schema said "set ARGENT_IOS_CAPTURE=device to override" without the "if the device path works on your host" caveat that the runtime error and IOS_PROFILER_REFERENCE.md both carry; on a genuinely degraded host the override still yields an empty trace. Match the sibling wording so the model-facing contract doesn't imply the override is a guaranteed fix.
hubgan
reviewed
Jul 9, 2026
hubgan
left a comment
hubgan
left a comment
Collaborator
There was a problem hiding this comment.
A few inline notes from a review + end-to-end pass on this branch (real iPhone 17 Pro, Xcode 26.6). The guard refusals, the malloc --launch argv, and the terminate/relaunch behavior all checked out live against the simulator — no correctness problems there. The comments below are the smaller things worth a look.
hubgan
approved these changes
Jul 9, 2026
An export can spell "no stack" more than one way for the same object type — a missing responsible-frame attribute parses as "Unknown" while truncated stacks emit the "<Call stack limit reached>" sentinel, and isLeakAttributed() trims before matching. aggregateLeaks() keyed on the verbatim frame, so those spellings split one unattributed group per type into several, inflating the "N unattributed leak group(s)" count. Key on the frame normalized exactly the way isLeakAttributed() matches it: trimmed, with every unattributed spelling collapsed to one bucket.
…loc_stack_logging The malloc_stack_logging description framed ARGENT_IOS_CAPTURE only as the =device escape hatch. An operator who keeps =all-processes exported globally for the normal capture path would hit the up-front rejection with no hint from the flag's own description; say so and name the remedy.
The unattributed-leaks hint inferred the capture mode from the attributed count, which is wrong in exactly one case: a malloc_stack_logging capture that attributed nothing (short capture, freed-region reuse, system-lib leaks) was told it ran via xctrace --attach and advised to enable the flag it just used. native-profiler-start now stamps the capture mode on the session (mallocStackLogging, null when unknown — Android, or a session restored from disk, which keeps the old inference as the fallback), and analyze threads it into the renderer. The hint block itself — previously a verbatim copy kept in sync by hand between render.ts and the combined report — is now a single shared renderUnattributedLeaksNote(), with a test pinning that both reports emit a byte-identical caveat line.
Every malloc-argv test let the notify mock throw, so registerStartupNotify returned null and --notify-tracing-started was never in the asserted argv — the `--launch -- <app>`-is-last guarantee was never exercised with the notify pair sitting in between, the shape a real host produces. And --no-prompt was unasserted in the malloc branch, which hand-rolls its argv apart from the strategy builder. Cover both; the production argv was already correct.
…ion from disk A live malloc_stack_logging capture stamps the session flag, and profiler-load (load_native) replaces exportedFiles/parsedData without touching it — so a session restored afterwards rendered the LOADED trace with the previous capture's mode, claiming "ran with malloc stack logging enabled" for a trace whose mode is unknown. The raw_*.xml carry no capture-mode sidecar, so null (unknown -> attributed-count inference) is the correct restored state. Repro'd via profiler-load on a session api with the flag set; regression test pins the reset.
renderLeakStacksIos carried the same unconditional "captured under xctrace --attach" framing the analyze report had: correct before malloc_stack_logging existed, wrong the moment a malloc capture leaves groups unattributed. Apply the same contract as the analyze/combined reports — use the session's real capture mode, fall back to inferring it from any attributed group in the FULL capture (deliberately not the object_type-filtered slice, so filtering out the attributed groups can't flip the note back to blaming --attach).
The session's mallocStackLogging was stamped at START, but exportedFiles is written at stop and parsedData at analyze — so a new recording re-labeled the previous capture's still-loaded data with its own mode. Concretely: analyze run mid-recording rendered a malloc capture's attributed table above a note claiming the capture used --attach, and leak_stacks/combined-report showed the inverse contradiction in the other ordering. Split the field: recordingMallocStackLogging is stamped at start (the in-flight capture), and mallocStackLogging flips only when stop writes exportedFiles (both the clean and the timed-out/unexpected-exit export paths). parsedData freezes the mode at analyze/load time so the drill-down consumers (leak_stacks, combined report) read the mode of the data they render, not the live session field. A failed start now leaves the previous capture's report-facing mode intact instead of nulling it. Also, per the same review pass: - profiler-load now clears ALL per-capture residue, not just the capture mode: a stale cpuFilterPid silently filtered the loaded trace's CPU samples by a dead PID, a stale wallClockStartMs fired the stale-trace note with the wrong timestamp, and a stale traceFile mislabeled the report and wrote the .md over the old trace's report. - The malloc-mode hint no longer lists 'allocations from before recording started' — impossible under a cold launch, where the process is created inside the recording; use the defensible causes (freed-region reuse, truncated stack logs, allocations outside the instrumented zones). - malloc-stack-logging.test.ts scrubs ARGENT_IOS_CAPTURE per test: four tests failed spuriously on hosts that export it globally — the exact setup the schema doc describes.
…grouping The Stage-1 correlate section still said leaks are grouped by objectType alone — stale since the aggregation moved to a composite key so distinct malloc_stack_logging call sites stay distinct, with the frame half normalized the way isLeakAttributed matches it.
…eak hints Three follow-ups from a second review pass over the branch: - profiler-load's new residue clearing nulled traceFile with no session-state guard, so load_native during an active recording wedged the session (stop threw NO_ACTIVE_SESSION while start threw SESSION_ALREADY_RUNNING) and orphaned the in-flight capture unexportable; a load after an unexpected xctrace exit likewise made stop's partial-trace recovery unreachable. load_native now refuses while profilingActive or a recovery flag is set, directing to native-profiler-stop first. - The unattributed-leaks hint let an explicit attach-mode flag override the attribution evidence: an app launched under MallocStackLogging externally (e.g. an Xcode scheme diagnostic) and then attached to rendered an attributed table directly above a 'no malloc-stack history' note. A recorded responsible frame proves the target ran under malloc stack logging regardless of how argent captured, so attributed>0 now decides first and the flag only disambiguates the zero-attributed case (both render.ts and the leak_stacks drill-down). - Per-capture descriptors (appProcess, traceFile, cpuFilterPid, capture mode) are now stamped only on a successful start: a failed attempt used to null them while the previous capture's exports stayed loaded, so a later analyze rendered a host-wide all-processes capture unfiltered and unlabeled.
…unch target Third review-pass follow-ups, each reproduced before fixing: - A FAILED native-profiler-start cleared recordingTimedOut / recordingExitedUnexpectedly before the attempt, so a capture that ended abnormally lost its pending partial-trace recovery to a mere failed restart (stop then threw NO_ACTIVE_SESSION and the trace was unrecoverable — only the raw_*.xml that stop's export writes are loadable). Nothing in the attempt reads those flags; reset them with the other per-capture stamps, on success only. - native-profiler-analyze now refuses while a recording is in flight or a crashed capture awaits recovery, mirroring the profiler-load guard: the live session fields belong to the newer capture, so the old exports would render under the new trace's name, freshness anchor, and CPU filter PID. - The malloc cold launch TERMINATES the resolved app, so resolveAppForLaunch no longer takes the first installed app matching app_process in plist order: an exact CFBundleExecutable match wins over display-name matches, and a display-name-only ambiguity (dev + prod builds both shown as "MyApp") is refused up front naming the candidates (NATIVE_PROFILER_LAUNCH_APP_AMBIGUOUS). - Leak tables now escape '|' in object type / responsible frame / library cells: GFM splits cells on unescaped pipes even inside code spans, and malloc_stack_logging makes demangled C++ frames (operator| included) the headline content of exactly these tables. - Docs: REFERENCE.md no longer claims frames appear 'only when recorded with malloc_stack_logging' (an externally launched MallocStackLogging app that argent attaches to also attributes); PIPELINE_DESIGN.md now describes the (objectType, attribution-normalized frame) aggregation and the malloc cold-launch capture mode.
… refusals, widen table escaping Pass-4 hardening of the malloc-attributable-leaks work: - profiler-combined-report now refuses while a capture is recording or a partial trace is pending recovery. It anchored the frozen parsedData hangs to the LIVE wallClockStartMs, so a capture started after analyze would shift every correlation by the gap between the two recordings' starts. Same contract analyze and profiler-load already enforce. - Factor the three in-flight refusals into a shared capture-guard helper that distinguishes recording vs 10-min-cap timeout vs unexpected exit, so the stated cause matches what native-profiler-stop reports next (a timeout is no longer mislabelled "ended unexpectedly"). - resolveAppForLaunch accepts a CFBundleIdentifier and the ambiguity refusal points at it: dev+prod builds sharing a CFBundleExecutable were an unresolvable dead end (the advice named the executable the user just passed). - startNativeProfilerAndroid starts perfetto before mutating session state, so a failed start no longer burns a prior capture's pending partial-trace recovery (the iOS start path already did this). - Escape '|' in the CPU-hotspot, function caller/callee, and thread-breakdown tables: dominantFunction is a real demangled frame in every capture mode, so a C++ operator| overload broke those rows regardless of malloc mode. - IOS_PROFILER_REFERENCE.md: the degraded-Xcode bound covers every 26.x from 26.4 up (26.5, 26.6, …), not only 26.4 and 27+.
…hread cells Two follow-ups to the pass-4 in-flight guard: - The iOS combined report anchored its FROZEN parsedData hangs to the LIVE session wallClockStartMs. A capture started after analyze re-stamps that live field while parsedData stays on the earlier capture, and native-profiler-stop refreshes neither — so re-running the report (including the path the guard's own recovery advice led to) shifted every hang by the gap between the two recordings' starts, silently dropping real correlations into "Hangs Without React Commit Match". Freeze the recording start into parsedData at analyze and anchor iOS off that frozen value; Android keeps its live anchor (it re-derives hangs from the live trace). The guard's recovery message now routes through native-profiler-analyze, which is the only step that rebuilds parsedData. - Escape '|' in the Thread column of the CPU-hotspot and thread-breakdown tables — the sibling Function column was escaped but a pipe in a dispatch-queue / thread name still misaligned the row.
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
2 participants
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.
What
Opt-in
malloc_stack_loggingflag onnative-profiler-startthat makes iOS memory leaks attributable — they come back with a real responsible frame + library instead of<Call stack limit reached>.Why
native-profiler-startattaches to the already-running app. iOS records malloc allocation backtraces only whenMallocStackLoggingis set at process launch, so an attached app has none: Instruments' Leaks scanner finds the leaked blocks but can't attribute them and emits the placeholder<Call stack limit reached>(that string is Instruments', not ours).How
When
malloc_stack_logging: true, the profiler cold-launches the app under xctrace instead of attaching:.apppath resolved viasimctl get_app_container; the running instance is terminated first for a clean cold start.malloc_stack_logging: true, instead of surfacing the raw placeholder.Evidence (real iPhone 16 sim, same scroll workload)
Before (attach) — every leak row:
After (
--env MallocStackLogging=1 --launch) — same Leaks export:itanium_demangle::OutputBuffer::grow(...)hermes::vm::JSTypedArrayBase::createBuffer(...)Tests
test/ios-instruments/malloc-stack-logging.test.ts— the malloc launch argv (--env MallocStackLogging=1,--launch -- <app>last, with and without--notify-tracing-started,--no-prompt) vs default--attach; terminate-first / get_app_container ordering; failed-start behavior; per-testARGENT_IOS_CAPTUREscrubbing.test/ios-instruments/leak-attribution-render.test.ts— unattributable vs attributed leak rendering, capture-mode-aware hints.test/ios-instruments/load-freshness.test.ts— per-capture residue clearing onprofiler-load, live/recovery-pending load refusals.test/ios-instruments/stop-recovery.test.ts— capture-mode pairing at stop (clean and recovery paths).test/profiler-combined-leaks.test.ts,test/profiler-leak-stacks.test.ts,test/ios-leak-pipeline.test.ts— shared caveat line, evidence-first hints, attribution-normalized grouping.test/ios-instruments/suite green (9 files / 70 tests); full tool-server suite green;tsc --noEmit+tsc -p tsconfig.test.jsonclean.Notes
E2E'd live twice during review (real iPhone 16 Pro / iPhone 17 Pro sims on Xcode 26.6): guard refusals fire before the app is touched, the
--devicecold launch's deadlock on degraded Xcode is confirmed (so the guard is warranted), and the malloc argv/terminate/relaunch behavior checked out on-device. Docs updated:IOS_PROFILER_REFERENCE.mdand theargent-native-profilerskill.Review follow-ups (post-approval hardening)
Later commits address the remaining review threads and several subsequent multi-lens review passes:
recordingMallocStackLogging), pairs it withexportedFilesat stop (mallocStackLogging), and freezes it intoparsedDataat analyze/load — so the analyze report, the combined report, and theleak_stacksdrill-down all name the capture mode of the data they actually render instead of inferring it (or mislabeling it after a newer capture starts). One sharedrenderUnattributedLeaksNote()keeps the wording identical across reports, and attribution evidence outranks the flag: a resolved frame proves the target ran under malloc stack logging however it was launched.profiler-loadrestores a clean slate. Loading clears all per-capture residue (capture mode,cpuFilterPid,wallClockStartMs,traceFile) so a loaded trace is never analyzed with a previous live capture's PID filter, freshness anchor, or report path — and refuses to load while a recording is in flight or a crashed capture awaitsnative-profiler-stop's partial-trace recovery, which the load would otherwise orphan.native-profiler-stop's partial-trace export — are stamped/reset only on a successful start, so a failed attempt leaves the previous capture's still-loaded exports fully described and a pending abnormal-end recovery intact.profiler-load,native-profiler-analyze, andprofiler-combined-reportall refuse while a recording is in flight or a crashed capture awaits recovery, via one sharedisCaptureInFlight/inFlightGuardMessagehelper whose message distinguishes recording vs 10-min-cap timeout vs unexpected exit (so the stated cause matches whatnative-profiler-stopreports next). Proceeding would otherwise wedge the session (load) or render one capture's frozen data under another's live fields (analyze/combined).parsedDataat analyze, and the iOS combined report anchors its hangs to that frozen value rather than the live session field — so a capture started after analyze can never shift a real hang↔commit correlation into "unmatched". Android keeps its live anchor (it re-derives hangs from the live trace). The combined-report guard's recovery advice routes throughnative-profiler-analyze, the only step that rebuildsparsedData.aggregateLeakskeys on(objectType, frame-normalized-the-way-isLeakAttributed-matches), so mixed unattributed spellings (Unknown, the sentinel, whitespace variants) can't split one group per type.app_processresolution prefers an exactCFBundleExecutablematch and, when builds still tie (dev + prod sharing an executable/display name), refuses and points at the globally-uniqueCFBundleIdentifier— which it also accepts as anapp_process— instead of killing whichever app the plist enumerates first.|. Demangled C++ frames (operator|included) are real headline content under malloc mode; every free-form cell across the leak, CPU-hotspot, function caller/callee, and thread-breakdown tables — symbol and thread/queue-name columns — is escaped so a stray pipe can't shift the row's columns.startNativeProfilerAndroidnow starts perfetto before mutating session state, mirroring iOS, so a failed start no longer burns a prior capped capture's pending partial-trace recovery.