Skip to content

Commit d1ee0de

Browse files
Preserve referenced environments from TTL eviction
TTL eviction ranked entries only by the sidecar lastUsedAt, which is refreshed on create/reuse but never when an environment is resolved for run, debug, or Pylance. A stable script's environment could therefore look stale and be evicted (clearing its association) while still in active use. Skip eviction of any cache entry a script association still references, so the sweep reclaims only orphaned entries (superseded by a dependency change, or left by a deleted/deselected script). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 83eb139 commit d1ee0de

2 files changed

Lines changed: 68 additions & 27 deletions

File tree

src/managers/builtin/inlineScript/envManager.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2933,8 +2933,19 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
29332933
const persistedAssociations = await this.getPersistedAssociationSnapshot();
29342934
const scriptPaths = this.getTrackedScriptPaths(persistedAssociations);
29352935
const priorSelections = this.getPriorSelections(scriptPaths);
2936+
// Never evict an environment that a script association still points to. `lastUsedAt` is only
2937+
// refreshed when an environment is created or reused (never when it is resolved for run, debug,
2938+
// or Pylance), so an actively-used environment can look stale here. Reclaim only orphaned entries
2939+
// (e.g. superseded by a dependency change, or left behind by a deleted or deselected script).
2940+
const referencedEnvDirs = this.getReferencedCacheEntryDirs(persistedAssociations, scriptPaths);
2941+
const evictableStaleEntries = staleEntries.filter(
2942+
(staleEntry) => !referencedEnvDirs.has(normalizePath(staleEntry)),
2943+
);
2944+
if (evictableStaleEntries.length === 0) {
2945+
return;
2946+
}
29362947
const removedCacheEntries = new Set<string>();
2937-
for (const staleEntry of staleEntries) {
2948+
for (const staleEntry of evictableStaleEntries) {
29382949
try {
29392950
const removed = await this.removeCacheEntryForClear(
29402951
cacheRoot,
@@ -3323,6 +3334,26 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
33233334
return fs.remove(entryPath);
33243335
}
33253336

3337+
private getReferencedCacheEntryDirs(
3338+
persistedAssociations: PersistedInlineScriptEnvironments,
3339+
scriptPaths: ReadonlySet<string>,
3340+
): Set<string> {
3341+
const referenced = new Set<string>();
3342+
for (const scriptPath of scriptPaths) {
3343+
const environmentPaths = [
3344+
persistedAssociations[scriptPath]?.environmentPath,
3345+
this.fsPathToPersistedAssociation.get(scriptPath)?.environmentPath,
3346+
this.fsPathToEnv.get(scriptPath)?.environmentPath.fsPath,
3347+
].filter((value): value is string => value !== undefined);
3348+
for (const environmentPath of environmentPaths) {
3349+
// Mirror isRemovedOrMissingCacheAssociation: the cache-entry dir is two levels above the
3350+
// interpreter executable (e.g. <envDir>/bin/python -> <envDir>).
3351+
referenced.add(normalizePath(path.dirname(path.dirname(environmentPath))));
3352+
}
3353+
}
3354+
return referenced;
3355+
}
3356+
33263357
private async getInvalidatedAssociationPaths(
33273358
scriptPaths: ReadonlySet<string>,
33283359
persistedAssociations: PersistedInlineScriptEnvironments,

src/test/managers/builtin/inlineScript/envManager.unit.test.ts

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5803,35 +5803,41 @@ suite('InlineScriptEnvManager', () => {
58035803
assert.strictEqual(await fs.pathExists(stale.sysPrefix), true);
58045804
});
58055805

5806-
test('invalidates associations and discovered environments for deleted entries', async () => {
5807-
const uri = scriptUri('associated.py');
5806+
test('removes discovered environments for evicted orphaned entries', async () => {
58085807
const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa');
58095808
await setLastUsedAt(stale, new Date(NOW.getTime() - TTL_MS - 1));
5810-
await manager.set(uri, stale);
58115809
(manager as unknown as { collection: PythonEnvironment[] }).collection = [stale];
5812-
const selectionListener = sinon.spy();
58135810
const collectionListener = sinon.spy();
5814-
manager.onDidChangeEnvironment(selectionListener);
58155811
manager.onDidChangeEnvironments(collectionListener);
58165812

58175813
assert.ok(await manager.create(scriptUri('trigger.py')));
58185814

5819-
assert.strictEqual(await manager.get(uri), undefined);
5820-
assert.strictEqual(persistedAssociations, undefined);
5815+
assert.strictEqual(await fs.pathExists(stale.sysPrefix), false);
58215816
assert.deepStrictEqual(await manager.getEnvironments('all'), []);
5822-
sinon.assert.calledOnce(selectionListener);
5823-
assert.strictEqual(
5824-
normalizePath(selectionListener.firstCall.args[0].uri.fsPath),
5825-
normalizePath(uri.fsPath),
5826-
);
5827-
assert.strictEqual(selectionListener.firstCall.args[0].old, stale);
5828-
assert.strictEqual(selectionListener.firstCall.args[0].new, undefined);
58295817
sinon.assert.calledOnceWithExactly(collectionListener, [
58305818
{ kind: EnvironmentChangeKind.remove, environment: stale },
58315819
]);
58325820
});
58335821

5834-
test('invalidates an association when another host removes the stale entry first', async () => {
5822+
test('preserves a stale entry and its association while a script still references it', async () => {
5823+
const uri = scriptUri('associated.py');
5824+
const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa');
5825+
await setLastUsedAt(stale, new Date(NOW.getTime() - TTL_MS - 1));
5826+
await manager.set(uri, stale);
5827+
(manager as unknown as { collection: PythonEnvironment[] }).collection = [stale];
5828+
const selectionListener = sinon.spy();
5829+
manager.onDidChangeEnvironment(selectionListener);
5830+
5831+
assert.ok(await manager.create(scriptUri('trigger.py')));
5832+
5833+
assert.strictEqual(await fs.pathExists(stale.sysPrefix), true);
5834+
assert.strictEqual(await manager.get(uri), stale);
5835+
assert.notStrictEqual(persistedAssociations, undefined);
5836+
assert.ok((await manager.getEnvironments('all')).some((env) => env === stale));
5837+
sinon.assert.notCalled(selectionListener);
5838+
});
5839+
5840+
test('does not attempt to remove a stale entry that a script still references', async () => {
58355841
const uri = scriptUri('associated.py');
58365842
const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa');
58375843
await setLastUsedAt(stale, new Date(NOW.getTime() - TTL_MS - 1));
@@ -5847,15 +5853,14 @@ suite('InlineScriptEnvManager', () => {
58475853
},
58485854
): Promise<string | undefined>;
58495855
};
5850-
sinon.stub(internalManager, 'removeCacheEntryForClear').callsFake(async () => {
5851-
await fs.remove(stale.sysPrefix);
5852-
return undefined;
5853-
});
5856+
const removeSpy = sinon.spy(internalManager, 'removeCacheEntryForClear');
58545857

58555858
assert.ok(await manager.create(scriptUri('trigger.py')));
58565859

5857-
assert.strictEqual(persistedAssociations, undefined);
5858-
assert.strictEqual(await manager.get(uri), undefined);
5860+
sinon.assert.notCalled(removeSpy);
5861+
assert.strictEqual(await fs.pathExists(stale.sysPrefix), true);
5862+
assert.strictEqual(await manager.get(uri), stale);
5863+
assert.notStrictEqual(persistedAssociations, undefined);
58595864
});
58605865

58615866
test('does not let an in-flight refresh re-add an evicted environment', async () => {
@@ -5913,14 +5918,19 @@ suite('InlineScriptEnvManager', () => {
59135918
});
59145919

59155920
test('does not fail creation when association cleanup cannot be persisted', async () => {
5916-
const uri = scriptUri('associated.py');
5917-
const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa');
5918-
await setLastUsedAt(stale, new Date(NOW.getTime() - TTL_MS - 1));
5919-
await manager.set(uri, stale);
5921+
const orphan = await createOwnedEnvironment('aaaaaaaaaaaaaaaa');
5922+
await setLastUsedAt(orphan, new Date(NOW.getTime() - TTL_MS - 1));
5923+
// A separate association whose environment was deleted out from under us. Evicting the
5924+
// orphaned entry above drives the association cleanup pass, and persisting that cleanup is
5925+
// what fails here.
5926+
const missingUri = scriptUri('missing.py');
5927+
const missing = await createOwnedEnvironment('bbbbbbbbbbbbbbbb');
5928+
await manager.set(missingUri, missing);
5929+
await fs.remove(missing.sysPrefix);
59205930
workspaceState.update.onSecondCall().rejects(new Error('Memento unavailable'));
59215931

59225932
assert.ok(await manager.create(scriptUri('trigger.py')));
5923-
assert.strictEqual(await fs.pathExists(stale.sysPrefix), false);
5933+
assert.strictEqual(await fs.pathExists(orphan.sysPrefix), false);
59245934
});
59255935
});
59265936

0 commit comments

Comments
 (0)