Skip to content

Commit 20c1381

Browse files
fix: clean up inline-script environments after activation (#1794)
## Summary Inline-script cache cleanup previously ran before the first environment creation in a window. Users who did not create another environment could keep old cached environments indefinitely, and cleanup could delay unrelated interpreter lookups. - Run one delayed activation sweep instead of triggering cleanup from `create()`. Keep the existing 14-day expiration policy, reclaim incomplete setups after a one-day grace period, and remove at most three entries per sweep. There is no daily sweep. - Keep candidate scans and cleanup-result publication outside the interpreter-lookup barrier. Retain per-entry locks, physical-path ownership checks, reference checks, and an under-lock age recheck before deletion. - Refresh cached environments' last-used timestamps on lookup. Keep optional bookkeeping off the critical path, use fail-fast read-path metadata writes, and withhold an old entry when its protection cannot be established rather than return an unsafe last-known descriptor. - Preserve the selected interpreter through temporary unavailability, expose setup for an explicit retry, and publish recovery events after bounded background revalidation. - Preserve retry deadlines and budgets across saves that leave the inline requirements unchanged. Cancel stale recovery when requirements or the stored association change, and recheck selection after asynchronous timestamp work. - Recover dead-owner cache locks and reconcile the environment catalog after deletion without discarding a replacement rebuilt at the same path. - Add regression coverage and document the cleanup and recovery behavior. The feature remains behind the existing `python-envs.inlineScripts.enabled` flag. This does not add a worker process, recurring cleanup, dependencies, or a new cache format. ## Validation Lint, type checking, the full unit suite, and the production bundle were repeated after integrating the latest `main` API-facade refactor and resolving the import in the relocated `src/extensionApi.ts`. Obsolete compiled files from the renamed modules were removed before recompilation. Cache-deletion, save/recovery, and ordinary-API scenarios were also rerun against the integrated implementation. - `npm run lint` - `npm run compile-tests` - `npm run unittest -- --reporter=dot --no-colors`: 2,409 passing, 6 pending. - Production webpack bundle. - Independent Windows walkthroughs using real Python virtual environments, cache metadata, and entry locks: - The actual 96-126 second activation timer removed an old orphan without calling the manager's `create()` and did not schedule another sweep. - Exactly three old orphaned entries were removed; referenced, recent, live-locked, newer-schema, and redirected entries were preserved. - A normal project `.venv` and the base Python installation remained usable. - Another window's use after eviction planning prevented deletion. - Temporary write failures and locks recovered with and without an intervening same-requirements save. - A pending timestamp write did not return a subsequently unset interpreter. - A healthy lookup completed while an unrelated cache inspection was deliberately paused. - HEAD/current shared-API comparisons retained ordinary-manager routing, venv/conda timeout fallbacks, explicit non-inline overrides, and neighboring-file isolation. The walkthroughs adapt the editor/discovery-service boundaries; they are not full live VS Code/Pylance GUI or macOS/Linux validation. Usage timestamps are not process-lifetime leases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c3bb837 commit 20c1381

15 files changed

Lines changed: 2395 additions & 120 deletions

File tree

‎docs/managing-python-projects.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ Stop runs or debug sessions using the environment before deleting it. The extens
113113

114114
On Windows, changing only the letter casing of a script's filename keeps its existing environment association. A rename does not validate unsaved dependency edits or install packages.
115115

116+
Unused cached script environments are cleaned up once per window, about two minutes after the extension activates, rather than being triggered by environment creation. Cleanup considers entries unused for more than 14 days and incomplete setups older than one day, removes at most three entries, and skips environments referenced by this workspace or entries it cannot safely inspect. There is no daily sweep.
117+
118+
The background cache scan does not hold up interpreter lookups. If another window briefly locks an entry, or its last-used time cannot be updated safely, the extension retries the script association in the background with bounded delays. Saving without changing the inline requirements does not cancel, postpone, or reset those retries; changing the requirements or stored inline-environment association cancels outdated recovery work. A temporarily unavailable selected environment keeps its association instead of silently switching execution to another interpreter, and the setup action is available for an explicit retry. If automatic recovery does not succeed, use that action to retry. Cleanup never installs packages.
119+
116120
## Assigning Environments to Projects
117121

118122
Each project can have its own Python environment. This is the core benefit of project management.

‎src/common/inlineScript/cacheLayout.ts‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import * as crypto from 'crypto';
55
import * as fsapi from 'fs-extra';
6+
import { rename as renameWithoutRetry } from 'fs/promises';
67
import * as path from 'path';
78
import { Uri } from 'vscode';
89
import type { PythonEnvironment } from '../../api';
@@ -258,11 +259,22 @@ async function inspectMetaJsonFile(metaPath: string): Promise<InlineScriptMetaRe
258259
* hold the cache-entry file lock, which serializes this operation across
259260
* extension-host processes.
260261
*/
261-
export function writeMetaJson(envDir: Uri, meta: InlineScriptEnvMeta): Promise<void> {
262+
export interface WriteMetaJsonOptions {
263+
/**
264+
* Fail immediately instead of letting graceful-fs retry a Windows sharing violation on the
265+
* rename for a full minute. Read-path bookkeeping must never hold the shared cache-entry lock
266+
* that long: other windows cannot tell it apart from a build or a deletion.
267+
*/
268+
readonly failFast?: boolean;
269+
}
270+
271+
export function writeMetaJson(envDir: Uri, meta: InlineScriptEnvMeta, options?: WriteMetaJsonOptions): Promise<void> {
262272
const finalPath = getMetaJsonPath(envDir).fsPath;
263273
const key = normalizePath(path.resolve(finalPath));
264274
const previous = pendingMetaJsonWrites.get(key) ?? Promise.resolve();
265-
const operation = previous.catch(() => undefined).then(() => writeMetaJsonOnce(envDir, meta, finalPath));
275+
const operation = previous
276+
.catch(() => undefined)
277+
.then(() => writeMetaJsonOnce(envDir, meta, finalPath, options?.failFast === true));
266278
let queued: Promise<void>;
267279
queued = operation.finally(() => {
268280
if (pendingMetaJsonWrites.get(key) === queued) {
@@ -273,7 +285,13 @@ export function writeMetaJson(envDir: Uri, meta: InlineScriptEnvMeta): Promise<v
273285
return queued;
274286
}
275287

276-
async function writeMetaJsonOnce(envDir: Uri, meta: InlineScriptEnvMeta, finalPath: string): Promise<void> {
288+
async function writeMetaJsonOnce(
289+
envDir: Uri,
290+
meta: InlineScriptEnvMeta,
291+
finalPath: string,
292+
failFast: boolean,
293+
): Promise<void> {
294+
const rename = failFast ? renameWithoutRetry : fsapi.rename;
277295
await fsapi.ensureDir(envDir.fsPath);
278296
const tmpSuffix = crypto.randomBytes(6).toString('hex');
279297
const tmpPath = `${finalPath}.tmp-${tmpSuffix}`;
@@ -285,18 +303,20 @@ async function writeMetaJsonOnce(envDir: Uri, meta: InlineScriptEnvMeta, finalPa
285303
try {
286304
await fsapi.writeFile(tmpPath, payload, 'utf8');
287305
try {
288-
await fsapi.rename(tmpPath, finalPath);
306+
await rename(tmpPath, finalPath);
289307
finalKnownToExist = true;
290308
return;
291309
} catch (err) {
292310
const code = (err as NodeJS.ErrnoException | undefined)?.code;
293-
if (!['EPERM', 'EEXIST', 'EBUSY'].includes(code ?? '')) {
311+
// Read-path bookkeeping must leave the old sidecar in place rather than enter
312+
// the backup/restore path, whose recovery may itself need a retrying rename.
313+
if (failFast || !['EPERM', 'EEXIST', 'EBUSY'].includes(code ?? '')) {
294314
throw err;
295315
}
296316
}
297317

298318
try {
299-
await fsapi.rename(finalPath, backupPath);
319+
await rename(finalPath, backupPath);
300320
hasBackup = true;
301321
} catch (err) {
302322
if (!isFileNotFoundError(err)) {
@@ -305,7 +325,7 @@ async function writeMetaJsonOnce(envDir: Uri, meta: InlineScriptEnvMeta, finalPa
305325
}
306326

307327
try {
308-
await fsapi.rename(tmpPath, finalPath);
328+
await rename(tmpPath, finalPath);
309329
finalKnownToExist = true;
310330
} catch (replaceError) {
311331
if (hasBackup) {

‎src/common/inlineScript/routingRegistry.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ interface ScriptRoutingState {
4141
readonly metadataIdentity?: string;
4242
readonly metadataRevision: number;
4343
readonly validatedAssociation: boolean;
44+
readonly environmentUnavailable?: boolean;
4445
}
4546

4647
export class InlineScriptRoutingRegistry implements Disposable {
@@ -49,6 +50,9 @@ export class InlineScriptRoutingRegistry implements Disposable {
4950
private readonly setupOutcomes = new Map<string, InlineScriptSetupOutcome>();
5051
private readonly _onDidChangeRouteability = new EventEmitter<InlineScriptRouteabilityChangeEvent>();
5152
private readonly _onDidChangeMetadata = new EventEmitter<InlineScriptMetadataChangeEvent>();
53+
private readonly _onDidChangeAvailability = new EventEmitter<Uri>();
54+
55+
public readonly onDidChangeAvailability: Event<Uri> = this._onDidChangeAvailability.event;
5256

5357
public readonly onDidChangeRouteability: Event<InlineScriptRouteabilityChangeEvent> =
5458
this._onDidChangeRouteability.event;
@@ -73,6 +77,8 @@ export class InlineScriptRoutingRegistry implements Disposable {
7377
metadataRevision,
7478
validatedAssociation:
7579
state.metadataIdentity === metadataIdentity ? state.validatedAssociation : false,
80+
environmentUnavailable:
81+
state.metadataIdentity === metadataIdentity ? state.environmentUnavailable : false,
7682
};
7783
},
7884
true,
@@ -129,9 +135,25 @@ export class InlineScriptRoutingRegistry implements Disposable {
129135
...state,
130136
uri: script instanceof Uri ? script : state.uri,
131137
validatedAssociation,
138+
environmentUnavailable: validatedAssociation ? state.environmentUnavailable : false,
132139
}));
133140
}
134141

142+
/** Keep temporary I/O failure separate from interpreter selection, while allowing setup to be retried. */
143+
public setEnvironmentUnavailable(uri: Uri, unavailable: boolean): void {
144+
const scriptPath = getInlineScriptRoutingKey(uri);
145+
if (scriptPath) {
146+
this.update(scriptPath, (state) => ({ ...state, uri, environmentUnavailable: unavailable }));
147+
}
148+
}
149+
150+
/** Whether a selected inline environment is temporarily withheld by a lookup. */
151+
public isEnvironmentUnavailable(uri: Uri): boolean {
152+
const scriptPath = getInlineScriptRoutingKey(uri);
153+
const state = scriptPath ? this.states.get(scriptPath) : undefined;
154+
return this.isRouteable(state) && state?.environmentUnavailable === true;
155+
}
156+
135157
public hasValidatedAssociation(script: Uri | string): boolean {
136158
const scriptPath = getInlineScriptRoutingKey(script);
137159
return scriptPath ? this.states.get(scriptPath)?.validatedAssociation === true : false;
@@ -177,6 +199,7 @@ export class InlineScriptRoutingRegistry implements Disposable {
177199
this.setupOutcomes.clear();
178200
this._onDidChangeMetadata.dispose();
179201
this._onDidChangeRouteability.dispose();
202+
this._onDidChangeAvailability.dispose();
180203
}
181204

182205
private update(
@@ -189,6 +212,7 @@ export class InlineScriptRoutingRegistry implements Disposable {
189212
validatedAssociation: false,
190213
};
191214
const previousRouteable = this.isRouteable(previous);
215+
const previouslyUnavailable = previousRouteable && previous.environmentUnavailable === true;
192216
const next = updater(previous);
193217

194218
if (!next.metadata && !next.validatedAssociation) {
@@ -214,6 +238,9 @@ export class InlineScriptRoutingRegistry implements Disposable {
214238
routeable,
215239
});
216240
}
241+
if (previouslyUnavailable !== (routeable && next.environmentUnavailable === true) && next.uri) {
242+
this._onDidChangeAvailability.fire(next.uri);
243+
}
217244
}
218245

219246
private isRouteable(state: ScriptRoutingState | undefined): boolean {

‎src/extensionApi.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import type {
3535
SetEnvironmentScope,
3636
} from './types';
3737
import { PackageVersionLookupNotSupportedError } from './publicErrors';
38+
import { INLINE_SCRIPT_MANAGER_ID } from './common/constants';
3839
import { traceError, traceInfo } from './common/logging';
3940
import { pickEnvironmentManager } from './common/pickers/managers';
4041
import { timeout } from './common/utils/asyncUtils';
@@ -269,6 +270,13 @@ export class PythonEnvironmentApiImpl implements PythonEnvironmentApi {
269270
// Keep the background resolution alive so the cache/last-known value gets populated and the
270271
// change event fires once it finishes.
271272
resolution.catch((ex) => traceError('Failed to resolve environment in background', ex));
273+
// Inline-script environments are reclaimed from a shared cache, and the manager withholds
274+
// one it cannot prove is still safe. Serving the last-known value here would hand back the
275+
// descriptor that decision just rejected, so only the timeout is skipped for them; every
276+
// other manager keeps the fast fallback.
277+
if (this.envManagers.getEnvironmentManager(currentScope)?.id === INLINE_SCRIPT_MANAGER_ID) {
278+
return resolution;
279+
}
272280
return this.envManagers.getLastKnownEnvironment(currentScope);
273281
}
274282
onDidChangeEnvironment: Event<DidChangeEnvironmentEventArgs> = this._onDidChangeEnvironment.event;

‎src/features/envManagers.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,13 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
233233
traceError('Failed to refresh inline-script routing:', error),
234234
);
235235
}),
236+
this.inlineScriptRouting.onDidChangeAvailability((uri) => {
237+
if (this.getEnvironmentManager(uri)?.id === INLINE_SCRIPT_MANAGER_ID) {
238+
void this.refreshEnvironment(uri, true).catch((error) =>
239+
traceError('Failed to refresh inline-script availability:', error),
240+
);
241+
}
242+
}),
236243
);
237244
}
238245
}
@@ -891,7 +898,11 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
891898
return undefined;
892899
}
893900

894-
return manager.get(scope);
901+
const environment = await manager.get(scope);
902+
if (manager.id === INLINE_SCRIPT_MANAGER_ID && this.getEnvironmentManager(scope) !== manager) {
903+
return this.getEnvironment(scope);
904+
}
905+
return environment;
895906
}
896907

897908
/**
@@ -902,8 +913,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
902913
*
903914
* Unlike getEnvironment(), this IS a mutation — it updates internal state.
904915
* Unlike setEnvironment(), it does NOT call manager.set() or persist to settings.
916+
* Availability recovery may republish an unchanged descriptor so consumers retry a failed lookup.
905917
*/
906-
async refreshEnvironment(scope: GetEnvironmentScope): Promise<void> {
918+
async refreshEnvironment(scope: GetEnvironmentScope, notifyIfUnchanged = false): Promise<void> {
907919
const manager = this.getEnvironmentManager(scope);
908920
if (!manager) {
909921
return;
@@ -918,7 +930,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
918930
}
919931

920932
const oldEnv = this._activeSelection.get(key);
921-
if (this.isSameEnvironment(oldEnv, newEnv) || !this.commitSelectionOperation(key, operation)) {
933+
if (
934+
(this.isSameEnvironment(oldEnv, newEnv) && !notifyIfUnchanged) ||
935+
!this.commitSelectionOperation(key, operation)
936+
) {
922937
return;
923938
}
924939
this._activeSelection.set(key, newEnv);

‎src/features/inlineScript/codeLens.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposabl
4646
) {
4747
this.subscriptions.push(
4848
this.routing.onDidChangeRouteability(() => this._onDidChangeCodeLenses.fire()),
49+
this.routing.onDidChangeAvailability(() => this._onDidChangeCodeLenses.fire()),
4950
// Only metadata arriving or changing can add/replace a lens; a scan that finds no metadata
5051
// (the common case for ordinary .py files) needs no refresh. Hiding a lens for an
5152
// edited/removed block is handled by VS Code re-querying on the document change itself.
@@ -89,7 +90,7 @@ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposabl
8990
const offset = metadata.sourceRange?.start ?? metadata.range.start;
9091
const position = document.positionAt(offset);
9192
const range = new Range(position, position);
92-
if (this.routing.shouldRoute(uri)) {
93+
if (this.routing.shouldRoute(uri) && !this.routing.isEnvironmentUnavailable(uri)) {
9394
// A validated inline-script environment matching the current metadata already exists.
9495
const key = getInlineScriptRoutingKey(uri);
9596
const confirmation = key ? this.readyConfirmations.get(key) : undefined;

‎src/features/inlineScript/setupCodeAction.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export class InlineScriptSetupCodeActionProvider implements CodeActionProvider {
9292
if (!getInlineScriptRoutingKey(uri)) {
9393
return [];
9494
}
95-
if (this.routing.shouldRoute(uri)) {
95+
if (this.routing.shouldRoute(uri) && !this.routing.isEnvironmentUnavailable(uri)) {
9696
return [];
9797
}
9898
if (!readInlineScriptMetadata(sliceHeaderBytes(document.getText()), uri.fsPath)) {

0 commit comments

Comments
 (0)