Skip to content

Commit 63d5a97

Browse files
committed
refactor(project): Prevent state corruption when source changes arrive during index re-init
After a SourceChangedDuringBuildError, the source index is nulled and state reset to RESTORING_PROJECT_INDICES. If projectSourcesChanged or dependencyResourcesChanged was called before initSourceIndex ran again, the state was incorrectly transitioned to REQUIRES_UPDATE, causing initSourceIndex to skip initialization and a subsequent NPE on the null source index.
1 parent 20a97a5 commit 63d5a97

2 files changed

Lines changed: 79 additions & 2 deletions

File tree

packages/project/lib/build/cache/ProjectBuildCache.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,8 @@ export default class ProjectBuildCache {
942942
this.#changedProjectSourcePaths.push(resourcePath);
943943
}
944944
}
945-
if (this.#combinedIndexState !== INDEX_STATES.INITIAL) {
945+
if (this.#combinedIndexState !== INDEX_STATES.INITIAL &&
946+
this.#combinedIndexState !== INDEX_STATES.RESTORING_PROJECT_INDICES) {
946947
// If there is an index cache, mark it as requiring update
947948
this.#combinedIndexState = INDEX_STATES.REQUIRES_UPDATE;
948949
}
@@ -962,7 +963,8 @@ export default class ProjectBuildCache {
962963
this.#changedDependencyResourcePaths.push(resourcePath);
963964
}
964965
}
965-
if (this.#combinedIndexState !== INDEX_STATES.INITIAL) {
966+
if (this.#combinedIndexState !== INDEX_STATES.INITIAL &&
967+
this.#combinedIndexState !== INDEX_STATES.RESTORING_PROJECT_INDICES) {
966968
// If there is an index cache, mark it as requiring update
967969
this.#combinedIndexState = INDEX_STATES.REQUIRES_UPDATE;
968970
}
@@ -1302,6 +1304,10 @@ export default class ProjectBuildCache {
13021304
* @throws {Error} If cached index signature doesn't match computed signature
13031305
*/
13041306
async #initSourceIndex() {
1307+
// Clear any pending source changes accumulated before initialization.
1308+
// The fresh disk read below already captures the current state.
1309+
this.#changedProjectSourcePaths = [];
1310+
13051311
const sourceReader = this.#project.getSourceReader();
13061312
const [resources, indexCache] = await Promise.all([
13071313
await sourceReader.byGlob("/**/*"),

packages/project/test/lib/build/cache/ProjectBuildCache.js

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,77 @@ test("projectSourcesChanged: tracks multiple changes", async (t) => {
741741
t.pass("Multiple changes tracked");
742742
});
743743

744+
test("projectSourcesChanged after SourceChangedDuringBuildError does not corrupt state", async (t) => {
745+
const project = createMockProject();
746+
const cacheManager = createMockCacheManager();
747+
748+
const resource = createMockResource("/test.js", "hash1", 1000, 100, 1);
749+
let byGlobCallCount = 0;
750+
project.getSourceReader.callsFake(() => ({
751+
byGlob: sinon.stub().callsFake(() => {
752+
byGlobCallCount++;
753+
if (byGlobCallCount === 1) {
754+
return Promise.resolve([resource]);
755+
}
756+
// On revalidation, return a modified resource to trigger SourceChangedDuringBuildError
757+
return Promise.resolve([createMockResource("/test.js", "hash2", 2000, 200, 2)]);
758+
}),
759+
byPath: sinon.stub().resolves(resource)
760+
}));
761+
762+
const indexCache = {
763+
version: "1.0",
764+
indexTree: {
765+
version: 1,
766+
indexTimestamp: 1000,
767+
root: {
768+
hash: "hash1",
769+
children: {
770+
"test.js": {
771+
hash: "hash1",
772+
metadata: {
773+
path: "/test.js",
774+
lastModified: 1000,
775+
size: 100,
776+
inode: 1
777+
}
778+
}
779+
}
780+
}
781+
},
782+
tasks: []
783+
};
784+
cacheManager.readIndexCache.resolves(indexCache);
785+
786+
const cache = await ProjectBuildCache.create(project, "sig", cacheManager);
787+
await cache.initSourceIndex();
788+
789+
// Simulate a build completing and detecting that sources changed during build
790+
// allTasksCompleted will throw SourceChangedDuringBuildError, setting #sourceIndex = null
791+
const error = await t.throwsAsync(() => cache.allTasksCompleted());
792+
t.true(error.message.includes("test.project"),
793+
"SourceChangedDuringBuildError thrown");
794+
795+
// Simulate what BuildServer does: flush resource changes before next build
796+
// This calls projectSourcesChanged while #sourceIndex is null
797+
cache.projectSourcesChanged(["/test.js"]);
798+
799+
// Now simulate next build attempt: initSourceIndex should still work
800+
// (state should still be RESTORING_PROJECT_INDICES, not REQUIRES_UPDATE)
801+
byGlobCallCount = 0; // Reset so initSourceIndex gets fresh resources
802+
await cache.initSourceIndex();
803+
804+
// And prepareProjectBuildAndValidateCache should not crash
805+
const mockDependencyReader = {
806+
byGlob: sinon.stub().resolves([]),
807+
byPath: sinon.stub().resolves(null)
808+
};
809+
await t.notThrowsAsync(
810+
() => cache.prepareProjectBuildAndValidateCache(mockDependencyReader),
811+
"prepareProjectBuildAndValidateCache does not throw after race condition"
812+
);
813+
});
814+
744815
test("prepareProjectBuildAndValidateCache: returns false for empty cache", async (t) => {
745816
const project = createMockProject();
746817
const cacheManager = createMockCacheManager();

0 commit comments

Comments
 (0)