Repository navigation
fix: fall back to a copy when a save's rename hits EXDEV #1256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; | ||
| import os from "node:os"; | ||
| import path from "node:path"; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
| import { renameSyncWithFallback, renameWithRetry } from "./atomic-rename"; | ||
|
|
||
| // No test machine has two layers of one folder, so the MSIX virtualization (#1255) is | ||
| // played by a rename that always answers EXDEV. | ||
| const exdev = vi.hoisted(() => ({ | ||
| on: false, | ||
| error: () => | ||
| Object.assign(new Error("EXDEV: cross-device link not permitted"), { code: "EXDEV" }), | ||
| })); | ||
|
|
||
| vi.mock("node:fs/promises", async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import("node:fs/promises")>(); | ||
| const rename: typeof actual.rename = (from, to) => | ||
| exdev.on ? Promise.reject(exdev.error()) : actual.rename(from, to); | ||
| return { ...actual, rename, default: { ...actual, rename } }; | ||
| }); | ||
|
|
||
| vi.mock("node:fs", async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import("node:fs")>(); | ||
| const renameSync: typeof actual.renameSync = (from, to) => { | ||
| if (exdev.on) throw exdev.error(); | ||
| actual.renameSync(from, to); | ||
| }; | ||
| return { ...actual, renameSync, default: { ...actual, renameSync } }; | ||
| }); | ||
|
|
||
| describe("atomic rename across an MSIX-virtualized folder", () => { | ||
| let dir: string; | ||
| let temp: string; | ||
| let target: string; | ||
|
|
||
| beforeEach(() => { | ||
| dir = mkdtempSync(path.join(os.tmpdir(), "atomic-rename-")); | ||
| temp = path.join(dir, "proj.openscreen.tmp-1"); | ||
| target = path.join(dir, "proj.openscreen"); | ||
| writeFileSync(temp, "new", "utf8"); | ||
| writeFileSync(target, "old", "utf8"); | ||
| exdev.on = true; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| exdev.on = false; | ||
| rmSync(dir, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| it("copies over the destination and removes the temp file", async () => { | ||
| await renameWithRetry(temp, target); | ||
| expect(readFileSync(target, "utf8")).toBe("new"); | ||
| expect(existsSync(temp)).toBe(false); | ||
| }); | ||
|
|
||
| it("does the same synchronously", () => { | ||
| renameSyncWithFallback(temp, target); | ||
| expect(readFileSync(target, "utf8")).toBe("new"); | ||
| expect(existsSync(temp)).toBe(false); | ||
| }); | ||
|
|
||
| it("never copies a case-only rename onto itself", async () => { | ||
| const preset = path.join(dir, "Studio.json"); | ||
| writeFileSync(preset, "preset", "utf8"); | ||
| await expect(renameWithRetry(preset, path.join(dir, "studio.json"))).rejects.toMatchObject({ | ||
| code: "EXDEV", | ||
| }); | ||
| expect(readFileSync(preset, "utf8")).toBe("preset"); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { copyFileSync, renameSync, rmSync } from "node:fs"; | ||
| import fs from "node:fs/promises"; | ||
|
|
||
| /** | ||
| * The Microsoft Store build is an MSIX package: Windows redirects its writes under AppData | ||
| * into the package's private copy of the folder. A rename between that copy and the real | ||
| * folder fails with EXDEV, even inside one directory (#1255). Copying then deleting is the | ||
| * only way across. It is not atomic, so it is taken only when the rename cannot be. A | ||
| * second staging copy beside the destination would not restore atomicity: the source | ||
| * already is one, and its rename is the one that failed. | ||
| * | ||
| * Never for a case-only rename: on a case-insensitive disk both names are the same file, | ||
| * and copying it onto itself then deleting the source would lose it. | ||
| */ | ||
| function mustCopyInstead(error: unknown, from: string, to: string): boolean { | ||
| return ( | ||
| (error as NodeJS.ErrnoException)?.code === "EXDEV" && from.toLowerCase() !== to.toLowerCase() | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * The rename half of a temp-then-rename save. Windows fails a rename onto an open file with | ||
| * EPERM/EBUSY: an indexer, an antivirus or a backup agent can hold the destination for a few | ||
| * milliseconds after we last touched it. The write itself is sound, so retry briefly rather | ||
| * than surface a save error the user cannot act on. POSIX renames don't hit this and take | ||
| * the first attempt. EXDEV falls back to a copy, see `mustCopyInstead`. | ||
| */ | ||
| export async function renameWithRetry(from: string, to: string): Promise<void> { | ||
| const RETRYABLE = new Set(["EPERM", "EACCES", "EBUSY"]); | ||
| for (let attempt = 0; ; attempt++) { | ||
| try { | ||
| await fs.rename(from, to); | ||
| return; | ||
| } catch (error) { | ||
| if (mustCopyInstead(error, from, to)) { | ||
| await fs.copyFile(from, to); | ||
| await fs.rm(from, { force: true }); | ||
| return; | ||
| } | ||
| const code = (error as NodeJS.ErrnoException)?.code ?? ""; | ||
| if (attempt >= 5 || !RETRYABLE.has(code)) throw error; | ||
| await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** `renameSync` with the same EXDEV fallback, for the saves that must stay synchronous. */ | ||
| export function renameSyncWithFallback(from: string, to: string): void { | ||
| try { | ||
| renameSync(from, to); | ||
| } catch (error) { | ||
| if (!mustCopyInstead(error, from, to)) throw error; | ||
| copyFileSync(from, to); | ||
| rmSync(from, { force: true }); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.