fix(simulator): make circuit recorder concurrency-safe via AsyncLocalStorage#24112
Merged
vezenovm merged 6 commits intoJun 16, 2026
Merged
Conversation
CircuitRecorder.finish() and finishWithError() dereferenced this.recording without a guard, so finalizing with no active recording threw "Cannot read properties of undefined (reading 'parent')". Because SimulatorRecorderWrapper calls finishWithError on the error path before re-throwing, that TypeError replaced the real failure (e.g. schnorr_initializerless: capsule load failed). Guard both methods (and the FileCircuitRecorder overrides) to resolve to undefined when there is no recording, so the original error propagates.
recordCall was the remaining unguarded this.recording! deref (companion to the finish/finishWithError guards): this.recording!.oracleCalls.push(entry). Under concurrent reuse of the shared recorder, this.recording can be reset mid-flight, so recordCall threw "Cannot read properties of undefined (reading 'oracleCalls')" AFTER the real oracle had already returned, fabricating a failure in an otherwise-successful execution (then surfaced via the wrapper's catch). Record best-effort and never throw: skip the push when there is no active recording. Also guards FileCircuitRecorder.recordCall and start (the file-recording-on path). The recorder is debug/profiling-only; making recordings concurrency-correct (no misparenting/dropping) is a separate follow-up.
vezenovm
changed the title
Thunkar
reviewed
Jun 16, 2026
| @@ -0,0 +1,9 @@ | |||
| import { readFileSync } from 'node:fs'; | |||
|
|
|||
| describe('client entrypoint', () => { | |||
Thunkar
approved these changes
Jun 16, 2026
Thunkar
left a comment
Thunkar
left a comment
Collaborator
There was a problem hiding this comment.
Much cleaner, but unfortunately browser incompatible. Still, this feature is relatively niche, so LGTM
mverzilli
approved these changes
Jun 16, 2026
Contributor
Author
Yeah unfortunately. This at least unblocks #23866 but we would need a bigger refactor which didn't feel necessary at this stage. |
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
3 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.
Holds the active circuit recording in
AsyncLocalStorageinstead of shared instance fields, so nested and concurrent executions no longer corrupt each other's recording.Problem:
CircuitRecorderkept mutable state (recording/stackDepth/newCircuit) on one per-PXE instance. Nested re-entrant executions (nested private calls viaaztec_prv_callPrivateFunction, utility executions) shared that state, so a child could reset the parent's recording mid-flight and drop or misattribute oracle calls. The error path then dereferenced the absent recording and masked the real failure (e.g.schnorr_initializerless: capsule load failed). The first two commits guarded the crash; this fixes the cause and leaves the guards as redundant defense.Fix:
AsyncLocalStorage(mirrorsfoundation/src/profiler/profiler.ts)record(metadata, fn)replacesstart/finish/finishWithErrorand re-throws the original error unchanged.CircuitSimulatorinterface or consumer change.Tests: oracle-call attribution across nested private + utility re-entries (red before this change: parent
oraclescame backundefined), isolation of interleaved concurrent recordings, and original simulator error surfacing without masking.Unblocks #23866