From af7a43963351c655eb86197bf52ae798a04e9c22 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 9 Oct 2026 21:01:53 +0800 Subject: [PATCH] fix(windows): retain runtime authority after cleanup failure --- native/windows/README.md | 4 +- native/windows/src/authority.hpp | 1 + native/windows/src/filesystem.cc | 10 +++-- scripts/verify-windows-native.mjs | 61 +++++++++++++++++++++++++++- src/windows/WindowsPrivateRuntime.ts | 14 +++---- 5 files changed, 77 insertions(+), 13 deletions(-) diff --git a/native/windows/README.md b/native/windows/README.md index d385dd48b..7fadb9b4e 100644 --- a/native/windows/README.md +++ b/native/windows/README.md @@ -57,7 +57,9 @@ without this lane reports Windows native controls as unavailable. deduplicated using Windows ordinal comparison, preserving their native Unicode semantics and caller-selected values. - Cleanup walks locked directory objects, removes files by handle, and unlinks - reparse entries without traversing their targets. Normal close, cancellation, + reparse entries without traversing their targets. Failed removal retains the + root and ancestor leases for retry; a partially removed runtime accepts only + cleanup, and its registry entry remains until removal succeeds. Normal close, cancellation, startup failure, and timeout release provider resources. Abrupt owner death kills the owned job; it can leave private runtime files for later operator cleanup and does not claim transactional deletion after a crash. diff --git a/native/windows/src/authority.hpp b/native/windows/src/authority.hpp index b4950ebbe..3c322da58 100644 --- a/native/windows/src/authority.hpp +++ b/native/windows/src/authority.hpp @@ -86,6 +86,7 @@ struct Runtime : Resource { std::vector parents; Handle directory; std::vector immutableFiles; + bool closing = false; // Main-thread admission is single-flight; only the accepted copy owns the // atomic cancellation flag consumed by its async worker. ERROR_BUSY rejects // another copy before it can reset that flag or replace pending ownership. diff --git a/native/windows/src/filesystem.cc b/native/windows/src/filesystem.cc index 8d7ef7a6b..4a3007623 100644 --- a/native/windows/src/filesystem.cc +++ b/native/windows/src/filesystem.cc @@ -328,14 +328,11 @@ static void disposeSnapshot(Runtime& root, Handle& file, const std::wstring& pat void closeRuntime(Runtime& root) { if (root.closed) return; + root.closing = true; // Windows keeps the bearer descriptor and snapshot leases until this owner // cleanup. POSIX removes its consumed descriptor earlier. Abrupt owner death // closes process jobs but cannot run this walk, so private files can remain. root.immutableFiles.clear(); - struct ReleaseRuntime { - Runtime& root; - ~ReleaseRuntime() { root.directory.reset(); root.parents.clear(); root.closed = true; } - } release{root}; // Each traversed directory stays locked against replacement. Reparse entries // are unlinked as objects; their targets are never enumerated or deleted. struct Frame { @@ -380,6 +377,10 @@ void closeRuntime(Runtime& root) { frames.push_back(std::make_unique(std::move(child), path)); else dispose(child.get(), path); } + // Failed walks retain the root's identity leases for a later cleanup retry. + root.directory.reset(); + root.parents.clear(); + root.closed = true; } struct SnapshotWork { @@ -528,6 +529,7 @@ napi_value filesystemCall(napi_env env, const std::wstring& operation, const std return result; } auto& root = static_cast(resource(env, args[0], Kind::Runtime)); + require(!root.closing || operation == L"runtime_close", "Runtime cleanup is pending", root.path, ERROR_BUSY); if (operation == L"runtime_snapshot_cancel") { root.snapshotCancelled.store(true); return null(env); } require(!root.snapshotPending, "Runtime snapshot is still pending", root.path, ERROR_BUSY); // Reject overlapping operations before snapshot() resets cancellation. The diff --git a/scripts/verify-windows-native.mjs b/scripts/verify-windows-native.mjs index 74d8f7cfe..289a1b384 100644 --- a/scripts/verify-windows-native.mjs +++ b/scripts/verify-windows-native.mjs @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import { execFileSync, spawn } from "node:child_process"; import { createHash } from "node:crypto"; +import { once } from "node:events"; import { access, mkdir, @@ -28,7 +29,7 @@ const { requireWindowsNativeAuthority } = await import( pathToFileURL(join(packageRoot, "dist/windows/WindowsNativeLoader.js")) ); const native = requireWindowsNativeAuthority(); -const { WindowsPrivateRuntime } = await import( +const { WindowsPrivateRuntime, windowsPrivateRuntime } = await import( pathToFileURL(join(packageRoot, "dist/windows/WindowsPrivateRuntime.js")) ); const workspace = await mkdtemp(join(tmpdir(), "rea-native-conformance-")); @@ -338,6 +339,64 @@ try { } finally { await readbackRuntime.close(); } + const retryRuntime = WindowsPrivateRuntime.create( + workspace, + "cleanup-retry-", + ); + const heldPath = join(retryRuntime.observation.path, "snapshot.bin"); + await retryRuntime.snapshot(source, "snapshot.bin"); + const holder = spawn( + "powershell.exe", + [ + "-NoProfile", + "-NonInteractive", + "-Command", + "$ErrorActionPreference = 'Stop'; " + + "$stream = [IO.File]::Open($env:REA_CLEANUP_HELD_PATH, [IO.FileMode]::Open, [IO.FileAccess]::Read, [IO.FileShare]::ReadWrite); " + + "try { [Console]::WriteLine('ready'); [Console]::In.ReadLine() | Out-Null } " + + "finally { $stream.Dispose() }", + ], + { + env: { ...process.env, REA_CLEANUP_HELD_PATH: heldPath }, + stdio: ["pipe", "pipe", "pipe"], + }, + ); + const holderClosed = once(holder, "close"); + void holderClosed.catch(() => undefined); + try { + const [ready] = await once(holder.stdout, "data", { + signal: AbortSignal.timeout(15_000), + }); + assert.equal(String(ready).trim(), "ready"); + await assert.rejects( + retryRuntime.close(), + /owned cleanup object|Delete owned object/u, + ); + assert.equal( + windowsPrivateRuntime(retryRuntime.observation.path), + retryRuntime, + ); + assert.equal(await exists(heldPath), true); + assert.throws( + () => retryRuntime.writeFile("after-close.txt", "forbidden"), + /cleanup is pending/u, + ); + holder.stdin.end("\n"); + const [exitCode] = await holderClosed; + assert.equal(exitCode, 0); + await retryRuntime.close(); + await retryRuntime.close(); + assert.equal(await exists(retryRuntime.observation.path), false); + assert.throws( + () => windowsPrivateRuntime(retryRuntime.observation.path), + /No native private runtime/u, + ); + report.controls.cleanupFailureRetainsOwnershipForRetry = true; + } finally { + holder.kill(); + await holderClosed.catch(() => undefined); + await retryRuntime.close(); + } report.controls = { ...report.controls, protectedDaclReadback: true, diff --git a/src/windows/WindowsPrivateRuntime.ts b/src/windows/WindowsPrivateRuntime.ts index 5d91dcfb8..24f876746 100644 --- a/src/windows/WindowsPrivateRuntime.ts +++ b/src/windows/WindowsPrivateRuntime.ts @@ -128,7 +128,10 @@ export class WindowsPrivateRuntime { /** Remove owned objects by handle, unlinking reparse entries without traversal. */ close(): Promise { - this.#closePromise ??= this.#remove(); + this.#closePromise ??= this.#remove().catch((cause: unknown) => { + this.#closePromise = undefined; + throw cause; + }); return this.#closePromise; } @@ -137,12 +140,9 @@ export class WindowsPrivateRuntime { if (this.#pending !== undefined) this.authority.call("runtime_snapshot_cancel", [this.observation.handle]); await this.#pending?.catch(() => undefined); - try { - this.authority.call("runtime_close", [this.observation.handle]); - } finally { - this.#closed = true; - runtimes.delete(this.observation.path.toLowerCase()); - } + this.authority.call("runtime_close", [this.observation.handle]); + this.#closed = true; + runtimes.delete(this.observation.path.toLowerCase()); } }