Visitar URL original
fix(core): avoid deleting idle bucket created during another bucket's… · angular/angular@b5208f9 · GitHub
Skip to content

Commit b5208f9

Browse files
Sepandardamishne
authored andcommitted
fix(core): avoid deleting idle bucket created during another bucket's drain
When a bucket was emptied and removed during its own drain, and a new bucket with the same key was created in the same drain, the end of the drain deleted the new bucket from the map instead of its own. The new bucket's idle request could then no longer be cancelled on removal or on destroy, and could run after the injector was destroyed (NG0205). Only delete the bucket from the map if it is still the one stored under that key. Fixes #70941 (cherry picked from commit 09e132a)
1 parent a23941f commit b5208f9

2 files changed

Lines changed: 136 additions & 1 deletion

File tree

‎packages/core/src/defer/idle_scheduler.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ export class IdleScheduler implements OnDestroy {
124124

125125
if (bucket.queue.size > 0) {
126126
this.scheduleBucket(bucket, options);
127-
} else {
127+
} else if (this.buckets.get(key) === bucket) {
128128
this.buckets.delete(key);
129129
}
130130
};

‎packages/core/test/acceptance/defer_spec.ts‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
provideZoneChangeDetection,
3535
QueryList,
3636
ɵRuntimeError as RuntimeError,
37+
signal,
3738
Type,
3839
ViewChild,
3940
ViewChildren,
@@ -829,6 +830,83 @@ describe('@defer', () => {
829830
globalThis.cancelIdleCallback = nativeCancelIdleCallback;
830831
}
831832
});
833+
834+
it('should cancel an `on idle` request registered while another idle callback was running', () => {
835+
@Component({
836+
selector: 'cmp-a',
837+
template: 'A',
838+
changeDetection: ChangeDetectionStrategy.Eager,
839+
})
840+
class CmpA {}
841+
842+
@Component({
843+
selector: 'cmp-b',
844+
template: 'B',
845+
changeDetection: ChangeDetectionStrategy.Eager,
846+
})
847+
class CmpB {}
848+
849+
@Component({
850+
selector: 'root-app',
851+
imports: [CmpA, CmpB],
852+
template: `
853+
@defer (on idle) {
854+
<cmp-a />
855+
}
856+
@if (showB()) {
857+
@defer (on idle) {
858+
<cmp-b />
859+
}
860+
}
861+
`,
862+
changeDetection: ChangeDetectionStrategy.Eager,
863+
})
864+
class RootCmp {
865+
showB = signal(false);
866+
}
867+
868+
@Injectable({providedIn: 'root'})
869+
class CustomIdleService implements IdleService {
870+
private nextId = 0;
871+
readonly pending = new Map<number, (deadline?: IdleDeadline) => void>();
872+
873+
requestOnIdle(callback: (deadline?: IdleDeadline) => void): number {
874+
const id = ++this.nextId;
875+
this.pending.set(id, callback);
876+
return id;
877+
}
878+
879+
cancelOnIdle(id: number): void {
880+
this.pending.delete(id);
881+
}
882+
883+
trigger(): void {
884+
const callbacks = [...this.pending.values()];
885+
this.pending.clear();
886+
callbacks.forEach((callback) => callback());
887+
}
888+
}
889+
890+
TestBed.configureTestingModule({
891+
providers: [...COMMON_PROVIDERS, provideIdleServiceWith(CustomIdleService)],
892+
});
893+
894+
const fixture = TestBed.createComponent(RootCmp);
895+
// Attach the view to `ApplicationRef` so that the tick inside the idle callback refreshes it.
896+
fixture.autoDetectChanges();
897+
898+
const idleService = TestBed.inject(CustomIdleService);
899+
expect(idleService.pending.size).toBe(1);
900+
901+
// Make the `@if` pending so that the tick inside the idle callback renders it.
902+
// Its block then requests a new idle callback while the first one is still running.
903+
fixture.componentInstance.showB.set(true);
904+
idleService.trigger();
905+
expect(idleService.pending.size).toBe(1);
906+
907+
fixture.destroy();
908+
expect(idleService.pending.size).toBe(0);
909+
});
832910
});
833911

834912
describe('directive matching', () => {
@@ -5649,4 +5727,61 @@ describe('IdleScheduler', () => {
56495727
expect(cbB).toHaveBeenCalledTimes(1);
56505728
expect(ricCount).toBe(1);
56515729
});
5730+
5731+
it('should cancel a bucket that was created while another bucket with the same key was draining', () => {
5732+
let capturedCbs: Array<(deadline: any) => void> = [];
5733+
let ricCount = 0;
5734+
5735+
customIdleService.requestOnIdleSpy.and.callFake((cb: any) => {
5736+
ricCount++;
5737+
capturedCbs.push(cb);
5738+
return 100 + ricCount;
5739+
});
5740+
const cancelOnIdleSpy = spyOn(customIdleService, 'cancelOnIdle');
5741+
5742+
const cbB = jasmine.createSpy('cbB');
5743+
const cbA = jasmine.createSpy('cbA').and.callFake(() => {
5744+
// Emulates a trigger cleanup emptying the current bucket, followed by
5745+
// a new idle-triggered block being registered within the same drain.
5746+
scheduler.remove(cbA);
5747+
scheduler.add(cbB);
5748+
});
5749+
5750+
scheduler.add(cbA);
5751+
capturedCbs[0]({didTimeout: false, timeRemaining: () => 10});
5752+
5753+
expect(cbA).toHaveBeenCalledTimes(1);
5754+
expect(cbB).toHaveBeenCalledTimes(0);
5755+
expect(ricCount).toBe(2);
5756+
5757+
cancelOnIdleSpy.calls.reset();
5758+
scheduler.remove(cbB);
5759+
expect(cancelOnIdleSpy).toHaveBeenCalledOnceWith(102);
5760+
});
5761+
5762+
it('should cancel a bucket created during another bucket drain on destroy', () => {
5763+
let capturedCbs: Array<(deadline: any) => void> = [];
5764+
let ricCount = 0;
5765+
5766+
customIdleService.requestOnIdleSpy.and.callFake((cb: any) => {
5767+
ricCount++;
5768+
capturedCbs.push(cb);
5769+
return 100 + ricCount;
5770+
});
5771+
const cancelOnIdleSpy = spyOn(customIdleService, 'cancelOnIdle');
5772+
5773+
const cbB = jasmine.createSpy('cbB');
5774+
const cbA = jasmine.createSpy('cbA').and.callFake(() => {
5775+
scheduler.remove(cbA);
5776+
scheduler.add(cbB);
5777+
});
5778+
5779+
scheduler.add(cbA);
5780+
capturedCbs[0]({didTimeout: false, timeRemaining: () => 10});
5781+
expect(ricCount).toBe(2);
5782+
5783+
cancelOnIdleSpy.calls.reset();
5784+
scheduler.ngOnDestroy();
5785+
expect(cancelOnIdleSpy).toHaveBeenCalledOnceWith(102);
5786+
});
56525787
});

0 commit comments

Comments
 (0)