Skip to content

Commit b3a73e4

Browse files
committed
feat: add local existence check for persisted paths in fast-path resolution
1 parent f71716d commit b3a73e4

2 files changed

Lines changed: 81 additions & 7 deletions

File tree

src/managers/common/fastPath.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
import * as fs from 'fs';
45
import { Uri } from 'vscode';
56
import { GetEnvironmentScope, PythonEnvironment, PythonEnvironmentApi } from '../../api';
67
import { traceError, traceVerbose, traceWarn } from '../../common/logging';
@@ -48,6 +49,28 @@ export function getProjectFsPathForScope(api: Pick<PythonEnvironmentApi, 'getPyt
4849
return api.getPythonProject(scope)?.uri.fsPath ?? scope.fsPath;
4950
}
5051

52+
/**
53+
* Cheap local existence check on the persisted path. If the cached interpreter has
54+
* been deleted/renamed/moved we can short-circuit before spawning PET (which holds
55+
* the cache mutex and can take up to the spawn timeout to fail).
56+
*
57+
* Returns true if the path exists (or if the check itself errors — in which case we
58+
* fall through to the normal resolve so we don't introduce a new failure mode).
59+
*/
60+
async function persistedPathExists(persistedPath: string): Promise<boolean> {
61+
try {
62+
await fs.promises.access(persistedPath);
63+
return true;
64+
} catch (err) {
65+
const code = (err as NodeJS.ErrnoException)?.code;
66+
if (code === 'ENOENT' || code === 'ENOTDIR') {
67+
return false;
68+
}
69+
// Unknown error (e.g. EACCES) — don't treat as missing; let resolve() decide.
70+
return true;
71+
}
72+
}
73+
5174
/**
5275
* Attempts fast-path resolution for manager.get(): if full initialization hasn't completed yet
5376
* and there's a persisted environment for the workspace, resolve it directly via nativeFinder
@@ -100,6 +123,18 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathRes
100123
const persistedPath = await getGlobalPersistedPath();
101124

102125
if (persistedPath) {
126+
// Cheap local existence check — avoids spawning PET (and waiting on the
127+
// bounded timeout) when the cached interpreter has been removed.
128+
if (!(await persistedPathExists(persistedPath))) {
129+
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch.elapsedTime, {
130+
managerLabel: opts.label,
131+
result: 'stale',
132+
});
133+
traceVerbose(
134+
`[${opts.label}] Fast path: persisted path '${persistedPath}' does not exist, falling through to slow path`,
135+
);
136+
return undefined;
137+
}
103138
try {
104139
const resolved = await opts.resolve(persistedPath);
105140
if (resolved) {
@@ -136,6 +171,14 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathRes
136171
const persistedPath = await opts.getPersistedPath(opts.getProjectFsPath(scope));
137172

138173
if (persistedPath) {
174+
// Cheap local existence check — avoids spawning PET (and waiting on the
175+
// bounded timeout) when the cached interpreter has been removed.
176+
if (!(await persistedPathExists(persistedPath))) {
177+
traceVerbose(
178+
`[${opts.label}] Fast path: persisted path '${persistedPath}' does not exist, falling through to slow path`,
179+
);
180+
return undefined;
181+
}
139182
try {
140183
const resolved = await opts.resolve(persistedPath);
141184
if (resolved) {

src/test/managers/common/fastPath.unit.test.ts

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ interface FastPathTestOptions {
3131

3232
function createOpts(overrides?: Partial<FastPathOptions>): FastPathTestOptions {
3333
const setInitialized = sinon.stub();
34-
const persistedPath = path.resolve('persisted', 'path');
34+
const persistedPath = __filename;
3535
return {
3636
opts: {
3737
initialized: undefined,
@@ -77,7 +77,7 @@ suite('tryFastPathGet', () => {
7777
});
7878

7979
test('returns resolved env for global scope when getGlobalPersistedPath returns a path', async () => {
80-
const globalPath = path.resolve('usr', 'bin', 'python3');
80+
const globalPath = __filename;
8181
const resolve = sinon.stub().resolves(createMockEnv(globalPath));
8282
const { opts } = createOpts({
8383
scope: undefined,
@@ -116,7 +116,7 @@ suite('tryFastPathGet', () => {
116116
});
117117

118118
test('reports stale when global cached path resolves to undefined', async () => {
119-
const globalPath = path.resolve('usr', 'bin', 'python3');
119+
const globalPath = __filename;
120120
const { opts } = createOpts({
121121
scope: undefined,
122122
getGlobalPersistedPath: sinon.stub().resolves(globalPath),
@@ -132,7 +132,7 @@ suite('tryFastPathGet', () => {
132132
});
133133

134134
test('returns undefined for global scope when cached path resolve fails', async () => {
135-
const globalPath = path.resolve('usr', 'bin', 'python3');
135+
const globalPath = __filename;
136136
const { opts } = createOpts({
137137
scope: undefined,
138138
getGlobalPersistedPath: sinon.stub().resolves(globalPath),
@@ -150,7 +150,7 @@ suite('tryFastPathGet', () => {
150150
});
151151

152152
test('global scope fast path starts background init when initialized is undefined', async () => {
153-
const globalPath = path.resolve('usr', 'bin', 'python3');
153+
const globalPath = __filename;
154154
const startBackgroundInit = sinon.stub().resolves();
155155
const { opts, setInitialized } = createOpts({
156156
scope: undefined,
@@ -201,6 +201,37 @@ suite('tryFastPathGet', () => {
201201
assert.strictEqual(result, undefined);
202202
});
203203

204+
test('skips resolve and falls through when workspace persisted path no longer exists on disk', async () => {
205+
const missingPath = path.resolve('does', 'not', 'exist', 'python-missing');
206+
const resolve = sinon.stub().resolves(createMockEnv(missingPath));
207+
const { opts } = createOpts({
208+
getPersistedPath: sinon.stub().resolves(missingPath),
209+
resolve,
210+
});
211+
const result = await tryFastPathGet(opts);
212+
213+
assert.strictEqual(result, undefined, 'Should fall through when cached path is missing');
214+
assert.ok(resolve.notCalled, 'Should not invoke resolve (and thus PET) when path is missing');
215+
});
216+
217+
test('skips resolve and reports stale when global persisted path no longer exists on disk', async () => {
218+
const missingPath = path.resolve('does', 'not', 'exist', 'python-missing');
219+
const resolve = sinon.stub().resolves(createMockEnv(missingPath));
220+
const { opts } = createOpts({
221+
scope: undefined,
222+
getGlobalPersistedPath: sinon.stub().resolves(missingPath),
223+
resolve,
224+
});
225+
const result = await tryFastPathGet(opts);
226+
227+
assert.strictEqual(result, undefined, 'Should fall through when cached global path is missing');
228+
assert.ok(resolve.notCalled, 'Should not invoke resolve (and thus PET) when path is missing');
229+
assert.ok(sendTelemetryStub.calledOnce, 'Should send telemetry for stale global cache');
230+
const [eventName, , props] = sendTelemetryStub.firstCall.args;
231+
assert.strictEqual(eventName, EventNames.GLOBAL_ENV_CACHE);
232+
assert.strictEqual(props.result, 'stale');
233+
});
234+
204235
test('calls getProjectFsPath with the scope Uri', async () => {
205236
const scope = Uri.file(path.resolve('my', 'project'));
206237
const getProjectFsPath = sinon.stub().returns(scope.fsPath);
@@ -214,7 +245,7 @@ suite('tryFastPathGet', () => {
214245
test('passes project fsPath to getPersistedPath', async () => {
215246
const projectPath = path.resolve('project', 'path');
216247
const getProjectFsPath = sinon.stub().returns(projectPath);
217-
const getPersistedPath = sinon.stub().resolves(path.resolve('persisted'));
248+
const getPersistedPath = sinon.stub().resolves(__filename);
218249
const { opts } = createOpts({
219250
getProjectFsPath,
220251
getPersistedPath,
@@ -266,7 +297,7 @@ suite('tryFastPathGet', () => {
266297
const getPersistedPath = sinon.stub().callsFake(
267298
() =>
268299
new Promise<string | undefined>((resolve) => {
269-
releasePersistedRead = () => resolve(path.resolve('persisted', 'path'));
300+
releasePersistedRead = () => resolve(__filename);
270301
}),
271302
);
272303
const { opts, setInitialized } = createOpts({ getPersistedPath });

0 commit comments

Comments
 (0)