Skip to content

Commit 600e56f

Browse files
refactor: trim PR scope to lock retirement, serialized clears, inline-key preservation
Reduce this PR's net diff versus its merge-base to only three logical changes: - #3 atomic canonical lock retirement on release (src/common/lockfile.apis.ts) - #6 serialized persistent-state clears (src/common/persistentState.ts) - #7 generic Clear Cache preserves the inline association key (constants.ts, envCommands.ts, extension.ts, and an import-only change in the inline-script envManager) Revert the other seven changes to the merge-base content: - #1 root-generation nonce, #2 root/entry admission decoupling, #9 create-counting order (inline-script envManager) - #4/#5 reclaim-side lock retirement + generation-specific inspection (keep only the minimal release-side companions inspect/reclaim need to stay correct: .release-* recognition and ENOENT-on-readdir tolerance) - #8 workspace-root protection (settingHelpers) - #10 typed ClearCacheNotSupported (envManagers, NotSupportedError) Also revert the associated test changes for the removed items, keeping the new persistentState suite (#6), the new Clear Environment Caches suite (#7), and the release-side lock-retirement tests (#3). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 4009eeb commit 600e56f

10 files changed

Lines changed: 76 additions & 911 deletions

File tree

src/common/errors/NotSupportedError.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,3 @@ export class RemoveEnvironmentNotSupported extends BaseError {
1111
super('NotSupported', message);
1212
}
1313
}
14-
15-
export class ClearCacheNotSupported extends BaseError {
16-
constructor(message: string) {
17-
super('NotSupported', message);
18-
}
19-
}

src/common/lockfile.apis.ts

Lines changed: 12 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ export interface AcquiredFileLock {
1919
export const FILE_LOCK_DIR_SUFFIX = '.lock';
2020
export const FILE_LOCK_OWNER_MARKER_PREFIX = 'owner-';
2121
export const FILE_LOCK_RETAINED_MARKER_PREFIX = 'retained-';
22-
export const FILE_LOCK_RECLAIM_MARKER_PREFIX = '.reclaim-';
2322
export const FILE_LOCK_RELEASE_MARKER_PREFIX = '.release-';
2423
export const FILE_LOCK_RETIRED_DIR_INFIX = '.retired-';
2524
/** Legacy retained marker. It remains recognizable but cannot be safely reclaimed. */
@@ -141,7 +140,7 @@ export async function inspectFileLock(filePath: string, options?: InspectFileLoc
141140
interface FileLockSnapshot {
142141
readonly state: FileLockState;
143142
readonly marker?: string;
144-
readonly markerKind?: 'owner' | 'retained' | 'reclaim' | 'release';
143+
readonly markerKind?: 'owner' | 'retained' | 'release';
145144
readonly generationMarker?: string;
146145
}
147146

@@ -176,14 +175,12 @@ async function inspectFileLockSnapshot(
176175
}
177176
const ownerEntries = entries.filter((entry) => entry.startsWith(FILE_LOCK_OWNER_MARKER_PREFIX));
178177
const generationRetainedEntries = entries.filter((entry) => entry.startsWith(FILE_LOCK_RETAINED_MARKER_PREFIX));
179-
const reclaimEntries = entries.filter((entry) => entry.startsWith(FILE_LOCK_RECLAIM_MARKER_PREFIX));
180178
const releaseEntries = entries.filter((entry) => entry.startsWith(FILE_LOCK_RELEASE_MARKER_PREFIX));
181179
const retainedEntries = entries.filter((entry) => entry === FILE_LOCK_RETAINED_MARKER);
182180
const unknownEntries = entries.filter(
183181
(entry) =>
184182
!entry.startsWith(FILE_LOCK_OWNER_MARKER_PREFIX) &&
185183
!entry.startsWith(FILE_LOCK_RETAINED_MARKER_PREFIX) &&
186-
!entry.startsWith(FILE_LOCK_RECLAIM_MARKER_PREFIX) &&
187184
!entry.startsWith(FILE_LOCK_RELEASE_MARKER_PREFIX) &&
188185
entry !== FILE_LOCK_RETAINED_MARKER,
189186
);
@@ -192,13 +189,11 @@ async function inspectFileLockSnapshot(
192189
unknownEntries.length > 0 ||
193190
ownerEntries.length > 1 ||
194191
generationRetainedEntries.length > 1 ||
195-
reclaimEntries.length > 1 ||
196192
releaseEntries.length > 1 ||
197193
retainedEntries.length > 1 ||
198-
(retainedEntries.length === 1 &&
199-
generationRetainedEntries.length + reclaimEntries.length + releaseEntries.length > 0) ||
194+
(retainedEntries.length === 1 && generationRetainedEntries.length + releaseEntries.length > 0) ||
200195
(retainedEntries.length === 0 &&
201-
ownerEntries.length + generationRetainedEntries.length + reclaimEntries.length + releaseEntries.length > 1)
196+
ownerEntries.length + generationRetainedEntries.length + releaseEntries.length > 1)
202197
) {
203198
return { state: 'malformed' };
204199
}
@@ -212,27 +207,6 @@ async function inspectFileLockSnapshot(
212207
}
213208
return { state: 'retained', marker: generationRetainedEntries[0], markerKind: 'retained' };
214209
}
215-
if (reclaimEntries.length === 1) {
216-
const reclaimMarker = parseTransitionMarker(reclaimEntries[0], FILE_LOCK_RECLAIM_MARKER_PREFIX);
217-
if (!reclaimMarker) {
218-
return { state: 'malformed' };
219-
}
220-
const liveness = await (options?.checkProcessLiveness ?? getProcessLiveness)(reclaimMarker.pid);
221-
if (liveness === 'dead') {
222-
return {
223-
state: 'stale',
224-
marker: reclaimEntries[0],
225-
markerKind: 'reclaim',
226-
generationMarker: reclaimMarker.generationMarker,
227-
};
228-
}
229-
return {
230-
state: liveness === 'live' ? 'held' : 'unavailable',
231-
marker: reclaimEntries[0],
232-
markerKind: 'reclaim',
233-
generationMarker: reclaimMarker.generationMarker,
234-
};
235-
}
236210
if (releaseEntries.length === 1) {
237211
const releaseMarker = parseTransitionMarker(releaseEntries[0], FILE_LOCK_RELEASE_MARKER_PREFIX);
238212
if (!releaseMarker) {
@@ -269,7 +243,7 @@ async function inspectFileLockSnapshot(
269243
}
270244

271245
/**
272-
* Claim the exact observed stale or retained generation, then atomically retire its canonical directory.
246+
* Claim and remove the exact observed stale or retained generation without releasing the lock directory.
273247
*/
274248
export async function reclaimFileLock(filePath: string, options?: InspectFileLockOptions): Promise<boolean> {
275249
const lockPath = getFileLockPath(filePath);
@@ -282,32 +256,29 @@ export async function reclaimFileLock(filePath: string, options?: InspectFileLoc
282256
return false;
283257
}
284258

285-
const generationMarker = snapshot.generationMarker ?? snapshot.marker;
286-
const claimedMarkerName =
287-
`${FILE_LOCK_RECLAIM_MARKER_PREFIX}${process.pid}-${crypto.randomBytes(16).toString('hex')}-${generationMarker}`;
288-
const claimedMarker = path.join(lockPath, claimedMarkerName);
289-
const observedMarker = path.join(lockPath, snapshot.marker);
259+
const claimedMarker = path.join(
260+
lockPath,
261+
`.reclaim-${process.pid}-${crypto.randomBytes(16).toString('hex')}-${snapshot.marker}`,
262+
);
290263
try {
291-
await fsapi.rename(observedMarker, claimedMarker);
264+
await fsapi.rename(path.join(lockPath, snapshot.marker), claimedMarker);
292265
} catch (error) {
293266
if (hasErrorCode(error, 'ENOENT') || hasErrorCode(error, 'EEXIST')) {
294267
return false;
295268
}
296269
throw error;
297270
}
298271

299-
const retiredPath = getRetiredLockPath(lockPath);
300272
try {
301-
await retireCanonicalLockDirectory(lockPath, retiredPath);
273+
await fsapi.unlink(claimedMarker);
274+
await fsapi.rmdir(lockPath);
275+
return true;
302276
} catch (error) {
303-
await fsapi.rename(claimedMarker, observedMarker).catch(() => undefined);
304277
if (hasErrorCode(error, 'ENOENT') || hasErrorCode(error, 'ENOTEMPTY')) {
305278
return false;
306279
}
307280
throw error;
308281
}
309-
await cleanupRetiredLock(retiredPath, claimedMarkerName);
310-
return true;
311282
}
312283

313284
export async function getProcessLiveness(pid: number): Promise<ProcessLiveness> {

src/features/envManagers.ts

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {
1515
EnvironmentManagerAlreadyRegisteredError,
1616
PackageManagerAlreadyRegisteredError,
1717
} from '../common/errors/AlreadyRegisteredError';
18-
import { ClearCacheNotSupported } from '../common/errors/NotSupportedError';
1918
import {
2019
InlineScriptRouteabilityChangeEvent,
2120
InlineScriptRoutingRegistry,
@@ -337,29 +336,12 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
337336
return;
338337
}
339338

340-
if (
341-
scope === INLINE_SCRIPT_MANAGER_ID ||
342-
(!(scope instanceof Uri) &&
343-
typeof scope !== 'string' &&
344-
scope.envId.managerId === INLINE_SCRIPT_MANAGER_ID)
345-
) {
346-
this.throwInlineClearNotSupported();
347-
}
348339
const manager = this.getEnvironmentManager(scope);
349-
if (manager?.id === INLINE_SCRIPT_MANAGER_ID) {
350-
this.throwInlineClearNotSupported();
351-
}
352-
if (manager) {
340+
if (manager && manager.id !== INLINE_SCRIPT_MANAGER_ID) {
353341
await manager.clearCache();
354342
}
355343
}
356344

357-
private throwInlineClearNotSupported(): never {
358-
throw new ClearCacheNotSupported(
359-
`Clear Cache for ${INLINE_SCRIPT_MANAGER_ID} requires the dedicated inline-script cache lifecycle.`,
360-
);
361-
}
362-
363345
/**
364346
* Sets the environment for a single scope, scope of undefined checks 'global'.
365347
* If given an array of scopes, delegates to setEnvironments for batch setting.

src/features/settings/settingHelpers.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -635,13 +635,9 @@ export async function removeInlineScriptPythonProjectSettings(
635635

636636
await Promise.all(promises);
637637

638-
const workspaceRootPaths = new Set(workspaceFolders.map((folder) => normalizePath(folder.uri.fsPath)));
639638
return Array.from(removedProjects.values())
640639
.map((project) => currentProjectsByUri.get(project.uri.toString()))
641-
.filter(
642-
(project): project is PythonProject =>
643-
project !== undefined && !workspaceRootPaths.has(normalizePath(project.uri.fsPath)),
644-
);
640+
.filter((project): project is PythonProject => project !== undefined);
645641
}
646642

647643
export async function addPythonProjectSetting(edits: EditProjectSettings[]): Promise<void> {

0 commit comments

Comments
 (0)