From ed20102713dd3269629b8e329f62d081be506430 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 7 Jun 2024 13:06:17 -0400 Subject: [PATCH] fix: make writing to the deps cache more reliable (#24135) I was able to reproduce this locally. ``` [error] Failed to execute snippet: import { validate } from "@std/uuid"; import { assert, assertFalse } from "@std/assert"; assert(validate("6ec0bd7f-11c0-43da-975e-2a8ad9ebae0b")); assertFalse(validate("not a UUID")); Download https://jsr.io/@std/uuid/meta.json Download https://jsr.io/@std/uuid/1.0.0-rc.1_meta.json Download https://jsr.io/@std/uuid/1.0.0-rc.1/mod.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/common.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/constants.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v1.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v3.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v4.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v5.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/_common.ts error: Access is denied. (os error 5) (for 'V:\.cache\deno\deps\https\jsr.io\2ae5bb614c7526d0876be0b76da1372fd51304ae27d6202ee94df720b3523d08') at file:///V:/deno_std/uuid/common.ts:43 [error] Failed to execute snippet: import { v5, NAMESPACE_DNS, NIL_UUID } from "@std/uuid"; import { assert, assertFalse } from "@std/assert"; const data = new TextEncoder().encode("deno.land"); const uuid = await v5.generate(NAMESPACE_DNS, data); assert(v5.validate(uuid)); assertFalse(v5.validate(NIL_UUID)); Download https://jsr.io/@std/uuid/meta.json Download https://jsr.io/@std/uuid/1.0.0-rc.1_meta.json Download https://jsr.io/@std/uuid/1.0.0-rc.1/mod.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/common.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/constants.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v1.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v3.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v4.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v5.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/_common.ts error: Access is denied. (os error 5) (for 'V:\.cache\deno\deps\https\jsr.io\63dd818c5fc1ac39c04df9b42bd9dd4bbc07f7d1b174e405d003731125778da1') at https://jsr.io/@std/uuid/1.0.0-rc.1/mod.ts:30:15 at file:///V:/deno_std/uuid/mod.ts:4 [error] Failed to execute snippet: import { isNil } from "@std/uuid"; import { assert, assertFalse } from "@std/assert"; assert(isNil("00000000-0000-0000-0000-000000000000")); assertFalse(isNil(crypto.randomUUID())); Download https://jsr.io/@std/uuid/meta.json Download https://jsr.io/@std/uuid/1.0.0-rc.1_meta.json Download https://jsr.io/@std/uuid/1.0.0-rc.1/mod.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/common.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/constants.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v1.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v3.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v4.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v5.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/_common.ts error: Access is denied. (os error 5) (for 'V:\.cache\deno\deps\https\jsr.io\fd3a12fc091d16ee29f10fa7a05eeeb8bd6c3cc014642e72478c757f00e7261e') at https://jsr.io/@std/uuid/1.0.0-rc.1/mod.ts:34:40 at file:///V:/deno_std/uuid/common.ts:23 [error] Failed to execute snippet: import { version } from "@std/uuid"; import { assertEquals } from "@std/assert/assert-equals"; assertEquals(version("d9428888-122b-11e1-b85c-61cd3cbb3210"), 1); assertEquals(version("6ec0bd7f-11c0-43da-975e-2a8ad9ebae0b"), 4); Download https://jsr.io/@std/uuid/meta.json Download https://jsr.io/@std/uuid/1.0.0-rc.1_meta.json Download https://jsr.io/@std/uuid/1.0.0-rc.1/mod.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/common.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/constants.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v1.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v3.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v4.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/v5.ts Download https://jsr.io/@std/uuid/1.0.0-rc.1/_common.ts error: Access is denied. (os error 5) (for 'V:\.cache\deno\deps\https\jsr.io\2ae5bb614c7526d0876be0b76da1372fd51304ae27d6202ee94df720b3523d08') at file:///V:/deno_std/uuid/common.ts:66 4 errors found ``` It occurs when many Deno processes are writing to the deps cache at the same time. Fix is to use `atomic_write_with_retries` which is much more reliable (and the function that helped make the ecosystem tests more reliable too). After this change I no longer have this issue. Closes https://github.com/denoland/deno/issues/24073 --- cli/cache/disk_cache.rs | 4 ++-- cli/cache/mod.rs | 4 ++-- cli/npm/managed/resolvers/local.rs | 4 ++-- cli/util/fs.rs | 8 +++++++- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/cli/cache/disk_cache.rs b/cli/cache/disk_cache.rs index 3aeebbc6d9..2fee1efe09 100644 --- a/cli/cache/disk_cache.rs +++ b/cli/cache/disk_cache.rs @@ -1,7 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use super::CACHE_PERM; -use crate::util::fs::atomic_write_file; +use crate::util::fs::atomic_write_file_with_retries; use deno_cache_dir::url_to_filename; use deno_core::url::Host; @@ -120,7 +120,7 @@ impl DiskCache { pub fn set(&self, filename: &Path, data: &[u8]) -> std::io::Result<()> { let path = self.location.join(filename); - atomic_write_file(&path, data, CACHE_PERM) + atomic_write_file_with_retries(&path, data, CACHE_PERM) } } diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index a8c60e97a2..64d046c153 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -7,7 +7,7 @@ use crate::file_fetcher::FetchOptions; use crate::file_fetcher::FileFetcher; use crate::file_fetcher::FileOrRedirect; use crate::npm::CliNpmResolver; -use crate::util::fs::atomic_write_file; +use crate::util::fs::atomic_write_file_with_retries; use deno_ast::MediaType; use deno_core::futures; @@ -74,7 +74,7 @@ impl deno_cache_dir::DenoCacheEnv for RealDenoCacheEnv { path: &Path, bytes: &[u8], ) -> std::io::Result<()> { - atomic_write_file(path, bytes, CACHE_PERM) + atomic_write_file_with_retries(path, bytes, CACHE_PERM) } fn modified(&self, path: &Path) -> std::io::Result> { diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs index 4243d04f2a..e93d400e16 100644 --- a/cli/npm/managed/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -17,7 +17,7 @@ use std::sync::Arc; use crate::cache::CACHE_PERM; use crate::npm::cache_dir::mixed_case_package_name_decode; -use crate::util::fs::atomic_write_file; +use crate::util::fs::atomic_write_file_with_retries; use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs; use crate::util::fs::clone_dir_recursive; use crate::util::fs::symlink_dir; @@ -550,7 +550,7 @@ impl SetupCache { } bincode::serialize(&self.current).ok().and_then(|data| { - atomic_write_file(&self.file_path, data, CACHE_PERM).ok() + atomic_write_file_with_retries(&self.file_path, data, CACHE_PERM).ok() }); true } diff --git a/cli/util/fs.rs b/cli/util/fs.rs index 5adb777b03..f33368d1a0 100644 --- a/cli/util/fs.rs +++ b/cli/util/fs.rs @@ -32,6 +32,12 @@ use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; use crate::util::progress_bar::ProgressMessagePrompt; +/// Writes the file to the file system at a temporary path, then +/// renames it to the destination in a single sys call in order +/// to never leave the file system in a corrupted state. +/// +/// This also handles creating the directory if a NotFound error +/// occurs. pub fn atomic_write_file_with_retries>( file_path: &Path, data: T, @@ -60,7 +66,7 @@ pub fn atomic_write_file_with_retries>( /// /// This also handles creating the directory if a NotFound error /// occurs. -pub fn atomic_write_file>( +fn atomic_write_file>( file_path: &Path, data: T, mode: u32,