fix(core): Explicitly manage TracingSnapshot lifecycle and dispose of it once it's been used. (#58929)

arielbackenroth · pkozlowski-opensource · commit 4792db9a6d3a · 2024-11-27T18:11:13.000+01:00
Provide a callback to the TracingService implementation when a Snapshot can be disposed. The underlying tracing implementation may use refcounting and needs to release resources to enable the trace to complete. While change detection uses the snapshot for exactly one callback, after render runs multiple hooks in the sequence so we need a more predictable way to indicate that the snapshot can be finalized.s PR Close #58929
diff --git a/packages/core/src/application/application_ref.ts b/packages/core/src/application/application_ref.ts
@@ -604,6 +604,7 @@ export class ApplicationRef {
       // if one exists. Snapshots may be reference counted by the implementation so
       // we want to ensure that if we request a snapshot that we use it.
       snapshot.run(TracingAction.CHANGE_DETECTION, this._tick);
+      snapshot.dispose();
       return;
     }
 
diff --git a/packages/core/src/application/tracing.ts b/packages/core/src/application/tracing.ts
@@ -17,6 +17,9 @@ export enum TracingAction {
 /** A single tracing snapshot. */
 export interface TracingSnapshot {
   run<T>(action: TracingAction, fn: () => T): T;
+
+  /** Disposes of the tracing snapshot. Must be run exactly once per TracingSnapshot. */
+  dispose(): void;
 }
 
 /**
@@ -39,6 +42,10 @@ export interface TracingService<T extends TracingSnapshot> {
    * used when additional work is performed that was scheduled in this context.
    *
    * @param linkedSnapshot Optional snapshot to use link to the current context.
+   * The caller is no longer responsible for calling dispose on the linkedSnapshot.
+   *
+   * @return The tracing snapshot. The caller is responsible for diposing of the
+   * snapshot.
    */
   snapshot(linkedSnapshot: T | null): T;
 }
diff --git a/packages/core/src/render3/after_render/manager.ts b/packages/core/src/render3/after_render/manager.ts
@@ -187,6 +187,7 @@ export class AfterRenderSequence implements AfterRenderRef {
     // associates the initial run of the hook with the context that created it.
     // Follow-up runs are independent of that initial context and have different
     // triggers.
+    this.snapshot?.dispose();
     this.snapshot = null;
   }
 
diff --git a/packages/core/test/acceptance/tracing_spec.ts b/packages/core/test/acceptance/tracing_spec.ts
@@ -27,6 +27,7 @@ describe('TracingService', () => {
         actions.push(action);
         return fn();
       },
+      dispose() {},
     };
     mockTracingService = {
       snapshot: jasmine.createSpy('snapshot').and.returnValue(fakeSnapshot),

Original file line number	Diff line number	Diff line change
`@@ -604,6 +604,7 @@ export class ApplicationRef {`
`604`	`604`	`// if one exists. Snapshots may be reference counted by the implementation so`
`605`	`605`	`// we want to ensure that if we request a snapshot that we use it.`
`606`	`606`	`snapshot.run(TracingAction.CHANGE_DETECTION, this._tick);`
	`607`	`+ snapshot.dispose();`
`607`	`608`	`return;`
`608`	`609`	`}`
`609`	`610`
Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,9 @@ export enum TracingAction {`
`17`	`17`	`/** A single tracing snapshot. */`
`18`	`18`	`export interface TracingSnapshot {`
`19`	`19`	`run<T>(action: TracingAction, fn: () => T): T;`
	`20`	`+`
	`21`	`+ /** Disposes of the tracing snapshot. Must be run exactly once per TracingSnapshot. */`
	`22`	`+ dispose(): void;`
`20`	`23`	`}`
`21`	`24`
`22`	`25`	`/**`
`@@ -39,6 +42,10 @@ export interface TracingService<T extends TracingSnapshot> {`
`39`	`42`	`* used when additional work is performed that was scheduled in this context.`
`40`	`43`	`*`
`41`	`44`	`* @param linkedSnapshot Optional snapshot to use link to the current context.`
	`45`	`+ * The caller is no longer responsible for calling dispose on the linkedSnapshot.`
	`46`	`+ *`
	`47`	`+ * @return The tracing snapshot. The caller is responsible for diposing of the`
	`48`	`+ * snapshot.`
`42`	`49`	`*/`
`43`	`50`	`snapshot(linkedSnapshot: T \| null): T;`
`44`	`51`	`}`
Original file line number	Diff line number	Diff line change
`@@ -187,6 +187,7 @@ export class AfterRenderSequence implements AfterRenderRef {`
`187`	`187`	`// associates the initial run of the hook with the context that created it.`
`188`	`188`	`// Follow-up runs are independent of that initial context and have different`
`189`	`189`	`// triggers.`
	`190`	`+ this.snapshot?.dispose();`
`190`	`191`	`this.snapshot = null;`
`191`	`192`	`}`
`192`	`193`