Skip to content

Commit a08913d

Browse files
Yi Cclaude
authored andcommitted
fix(rendering): update BlitDesc.blit on reuse to prevent stale-reference crash (#18884)
When DeviceRenderQueue.createBlitDesc() reused an existing _blitDesc, it did not update the stored blit reference. This caused BlitDesc.createStageDescriptor() to read a stale Blit whose material/passID could be invalid, resulting in "Cannot read 'localSetLayout' of undefined" — an infinite error loop that froze the editor. The fix updates this._blitDesc.blit = blit in the reuse path before calling createScreenQuad() and createStageDescriptor(). Closes #18884 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 411f98d commit a08913d

2 files changed

Lines changed: 198 additions & 0 deletions

File tree

cocos/rendering/custom/executor.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,8 @@ class DeviceRenderQueue implements RecordingInterface {
618618
createBlitDesc (blit: Blit): void {
619619
if (!this._blitDesc) {
620620
this._blitDesc = new BlitDesc(blit);
621+
} else {
622+
this._blitDesc.blit = blit;
621623
}
622624
this._blitDesc.createScreenQuad();
623625
this._blitDesc.createStageDescriptor();
Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
/**
2+
* Verification test for the BlitDesc stale-reference fix.
3+
*
4+
* Bug: cocos/cocos-engine#18884
5+
* When DeviceRenderQueue.createBlitDesc() reuses an existing _blitDesc,
6+
* the old code did NOT update _blitDesc.blit, so createStageDescriptor()
7+
* would read a stale Blit whose passID indexes into the wrong material's
8+
* passes array → undefined → "Cannot read 'localSetLayout' of undefined".
9+
*
10+
* This test simulates the exact crash path without needing a GPU context.
11+
*/
12+
13+
// ---- Minimal mocks matching the real types just enough to exercise the path ----
14+
15+
interface MockPass {
16+
localSetLayout: object;
17+
}
18+
19+
interface MockMaterial {
20+
passes: (MockPass | undefined)[];
21+
}
22+
23+
interface MockBlit {
24+
material: MockMaterial | null;
25+
passID: number;
26+
sceneFlags: number;
27+
camera: null;
28+
}
29+
30+
// Simulates BlitDesc.createStageDescriptor (executor.ts:486-490)
31+
function createStageDescriptor (blit: MockBlit): MockPass {
32+
const pass = blit.material!.passes[blit.passID];
33+
// This is the exact line that crashes when pass is undefined:
34+
const _layout = pass!.localSetLayout;
35+
return pass!;
36+
}
37+
38+
// ---- The test scenarios ----
39+
40+
function runTests (): { passed: number; failed: number } {
41+
let passed = 0;
42+
let failed = 0;
43+
44+
function assert (condition: boolean, msg: string): void {
45+
if (condition) {
46+
console.log(` ✓ ${msg}`);
47+
passed++;
48+
} else {
49+
console.error(` ✗ ${msg}`);
50+
failed++;
51+
}
52+
}
53+
54+
// --- Scenario: Demonstrate the crash with stale blit (pre-fix behavior) ---
55+
console.log('\n[Test 1] Stale blit reference causes crash (pre-fix behavior)');
56+
{
57+
const materialA: MockMaterial = {
58+
passes: [{ localSetLayout: {} }], // 1 pass, passID=0 is valid
59+
};
60+
const materialB: MockMaterial = {
61+
passes: [], // 0 passes, any passID is out of bounds
62+
};
63+
const blitA: MockBlit = { material: materialA, passID: 0, sceneFlags: 0, camera: null };
64+
const blitB: MockBlit = { material: materialB, passID: 0, sceneFlags: 0, camera: null };
65+
66+
// First call: blitA works fine
67+
let firstCallOk = false;
68+
try {
69+
createStageDescriptor(blitA);
70+
firstCallOk = true;
71+
} catch (_e) {
72+
firstCallOk = false;
73+
}
74+
assert(firstCallOk, 'blitA (valid material) succeeds');
75+
76+
// Simulate pre-fix: reuse without updating blit → still reads blitA
77+
// But if we pass blitB directly, it crashes because materialB.passes[0] is undefined
78+
let crashedWithBlitB = false;
79+
try {
80+
createStageDescriptor(blitB);
81+
} catch (_e) {
82+
crashedWithBlitB = true;
83+
}
84+
assert(crashedWithBlitB, 'blitB (empty passes) crashes as expected — this is the bug scenario');
85+
}
86+
87+
// --- Scenario: Verify the fix ensures blit is updated before createStageDescriptor ---
88+
console.log('\n[Test 2] Fix: updating blit on reuse prevents stale reference');
89+
{
90+
const materialA: MockMaterial = {
91+
passes: [{ localSetLayout: {} }],
92+
};
93+
const materialB: MockMaterial = {
94+
passes: [{ localSetLayout: {} }, { localSetLayout: {} }],
95+
};
96+
const blitA: MockBlit = { material: materialA, passID: 0, sceneFlags: 0, camera: null };
97+
const blitB: MockBlit = { material: materialB, passID: 1, sceneFlags: 0, camera: null };
98+
99+
// Simulate the fixed createBlitDesc logic (executor.ts:618-625)
100+
let storedBlit: MockBlit = blitA; // constructor sets _blit = blitA
101+
102+
// Second call: the fix updates storedBlit before createStageDescriptor
103+
storedBlit = blitB; // this._blitDesc.blit = blit (line 622)
104+
105+
let success = false;
106+
try {
107+
const pass = createStageDescriptor(storedBlit);
108+
success = pass !== undefined && pass.localSetLayout !== undefined;
109+
} catch (_e) {
110+
success = false;
111+
}
112+
assert(success, 'After blit update, createStageDescriptor uses correct material/passID');
113+
114+
// Verify it would crash WITHOUT the update (stale blitA + blitB's passID scenario)
115+
const blitC: MockBlit = { material: materialA, passID: 1, sceneFlags: 0, camera: null };
116+
// materialA only has 1 pass (index 0), passID=1 is out of bounds
117+
let wouldCrash = false;
118+
try {
119+
createStageDescriptor(blitC);
120+
} catch (_e) {
121+
wouldCrash = true;
122+
}
123+
assert(wouldCrash, 'Without update: stale material + new passID → crash (proves fix is needed)');
124+
}
125+
126+
// --- Scenario: Simulate full DeviceRenderQueue.createBlitDesc flow ---
127+
// The real crash: render graph calls createBlitDesc(blitA) then createBlitDesc(blitB).
128+
// Between calls, blitA's material gets destroyed (passes cleared). BROKEN keeps stale
129+
// reference to blitA → createStageDescriptor reads destroyed material → crash.
130+
// FIXED updates to blitB → reads valid material → no crash.
131+
console.log('\n[Test 3] Full createBlitDesc reuse — stale blit with destroyed material');
132+
{
133+
let _blitDesc: { blit: MockBlit } | null = null;
134+
135+
function createBlitDesc_FIXED (blit: MockBlit): void {
136+
if (!_blitDesc) {
137+
_blitDesc = { blit };
138+
} else {
139+
_blitDesc.blit = blit; // THE FIX (line 622)
140+
}
141+
createStageDescriptor(_blitDesc.blit);
142+
}
143+
144+
function createBlitDesc_BROKEN (blit: MockBlit): void {
145+
if (!_blitDesc) {
146+
_blitDesc = { blit };
147+
}
148+
// Missing: _blitDesc.blit = blit
149+
createStageDescriptor(_blitDesc!.blit);
150+
}
151+
152+
const mat1: MockMaterial = { passes: [{ localSetLayout: {} }] };
153+
const mat2: MockMaterial = { passes: [{ localSetLayout: {} }] };
154+
const blit1: MockBlit = { material: mat1, passID: 0, sceneFlags: 0, camera: null };
155+
const blit2: MockBlit = { material: mat2, passID: 0, sceneFlags: 0, camera: null };
156+
157+
// BROKEN: first call stores blit1, then material is destroyed, second call still reads blit1
158+
_blitDesc = null;
159+
createBlitDesc_BROKEN(blit1); // stores blit1
160+
mat1.passes.length = 0; // simulate material destruction / pass invalidation
161+
let brokenCrashed = false;
162+
try {
163+
createBlitDesc_BROKEN(blit2); // BROKEN: still reads blit1 → mat1.passes[0] = undefined → crash
164+
} catch (_e) {
165+
brokenCrashed = true;
166+
}
167+
assert(brokenCrashed, 'BROKEN: stale blit with destroyed material crashes');
168+
169+
// FIXED: same scenario, but blit is updated → reads blit2 with valid mat2
170+
_blitDesc = null;
171+
mat1.passes.push({ localSetLayout: {} }); // restore mat1 for first call
172+
createBlitDesc_FIXED(blit1);
173+
mat1.passes.length = 0; // destroy mat1 again
174+
let fixedOk = true;
175+
try {
176+
createBlitDesc_FIXED(blit2); // FIXED: updates to blit2 → reads mat2.passes[0] ✓
177+
} catch (_e) {
178+
fixedOk = false;
179+
}
180+
assert(fixedOk, 'FIXED: updated blit with valid material succeeds');
181+
}
182+
183+
return { passed, failed };
184+
}
185+
186+
// ---- Run ----
187+
console.log('=== BlitDesc stale-reference fix verification ===');
188+
console.log('Bug: cocos/cocos-engine#18884');
189+
console.log('File: cocos/rendering/custom/executor.ts:618-625');
190+
191+
const { passed, failed } = runTests();
192+
193+
console.log(`\n=== Results: ${passed} passed, ${failed} failed ===`);
194+
if (failed > 0) {
195+
process.exit(1);
196+
}

0 commit comments

Comments
 (0)