Merge pull request #183397 from microsoft/ben/uniform-gayal

Provide atomic write and delete support in file service (fix #182974)

src/vs/base/node/pfs.ts

@@ -37,8 +37,13 @@ export enum RimRafMode {
  * - `UNLINK`: direct removal from disk
  * - `MOVE`: faster variant that first moves the target to temp dir and then
  *           deletes it in the background without waiting for that to finish.
+ *           the optional `moveToPath` allows to override where to rename the
+ *           path to before deleting it.
  */
-async function rimraf(path: string, mode = RimRafMode.UNLINK): Promise<void> {
+async function rimraf(path: string, mode: RimRafMode.UNLINK): Promise<void>;
+async function rimraf(path: string, mode: RimRafMode.MOVE, moveToPath?: string): Promise<void>;
+async function rimraf(path: string, mode?: RimRafMode, moveToPath?: string): Promise<void>;
+async function rimraf(path: string, mode = RimRafMode.UNLINK, moveToPath?: string): Promise<void> {
 	if (isRootOrDriveLetter(path)) {
 		throw new Error('rimraf - will refuse to recursively delete root');
 	}
@@ -49,12 +54,11 @@ async function rimraf(path: string, mode = RimRafMode.UNLINK): Promise<void> {
 	}
 
 	// delete: via move
-	return rimrafMove(path);
+	return rimrafMove(path, moveToPath);
 }
 
-async function rimrafMove(path: string): Promise<void> {
+async function rimrafMove(path: string, moveToPath = randomPath(tmpdir())): Promise<void> {
 	try {
-		const pathInTemp = randomPath(tmpdir());
 		try {
 			// Intentionally using `fs.promises` here to skip
 			// the patched graceful-fs method that can result
@@ -64,7 +68,7 @@ async function rimrafMove(path: string): Promise<void> {
 			// than necessary and we have a fallback to delete
 			// via unlink.
 			// https://github.com/microsoft/vscode/issues/139908
-			await fs.promises.rename(path, pathInTemp);
+			await fs.promises.rename(path, moveToPath);
 		} catch (error) {
 			if (error.code === 'ENOENT') {
 				return; // ignore - path to delete did not exist
@@ -74,7 +78,7 @@ async function rimrafMove(path: string): Promise<void> {
 		}
 
 		// Delete but do not return as promise
-		rimrafUnlink(pathInTemp).catch(error => {/* ignore */ });
+		rimrafUnlink(moveToPath).catch(error => {/* ignore */ });
 	} catch (error) {
 		if (error.code !== 'ENOENT') {
 			throw error;

src/vs/base/test/node/pfs/pfs.test.ts

@@ -10,7 +10,7 @@ import { timeout } from 'vs/base/common/async';
 import { VSBuffer } from 'vs/base/common/buffer';
 import { randomPath } from 'vs/base/common/extpath';
 import { FileAccess } from 'vs/base/common/network';
-import { join, sep } from 'vs/base/common/path';
+import { basename, dirname, join, sep } from 'vs/base/common/path';
 import { isWindows } from 'vs/base/common/platform';
 import { configureFlushOnWrite, Promises, RimRafMode, rimrafSync, SymlinkSupport, writeFileSync } from 'vs/base/node/pfs';
 import { flakySuite, getRandomTestPath } from 'vs/base/test/node/testUtils';
@@ -91,6 +91,14 @@ flakySuite('PFS', function () {
 		assert.ok(!fs.existsSync(testDir));
 	});
 
+	test('rimraf - simple - move (with moveToPath)', async () => {
+		fs.writeFileSync(join(testDir, 'somefile.txt'), 'Contents');
+		fs.writeFileSync(join(testDir, 'someOtherFile.txt'), 'Contents');
+
+		await Promises.rm(testDir, RimRafMode.MOVE, join(dirname(testDir), `${basename(testDir)}.vsctmp`));
+		assert.ok(!fs.existsSync(testDir));
+	});
+
 	test('rimraf - path does not exist - move', async () => {
 		const nonExistingDir = join(testDir, 'unknown-move');
 		await Promises.rm(nonExistingDir, RimRafMode.MOVE);

src/vs/platform/files/browser/htmlFileSystemProvider.ts

@@ -269,8 +269,8 @@ export class HTMLFileSystemProvider implements IFileSystemProviderWithFileReadWr
 				const file = await fileHandle.getFile();
 				const contents = new Uint8Array(await file.arrayBuffer());
 
-				await this.writeFile(to, contents, { create: true, overwrite: opts.overwrite, unlock: false });
-				await this.delete(from, { recursive: false, useTrash: false });
+				await this.writeFile(to, contents, { create: true, overwrite: opts.overwrite, unlock: false, atomic: false });
+				await this.delete(from, { recursive: false, useTrash: false, atomic: false });
 			}
 
 			// File API does not support any real rename otherwise

src/vs/platform/files/browser/indexedDBFileSystemProvider.ts

@@ -311,7 +311,7 @@ export class IndexedDBFileSystemProvider extends Disposable implements IFileSyst
 				throw createFileSystemProviderError('Cannot rename files with different types', FileSystemProviderErrorCode.Unknown);
 			}
 			// delete the target file if exists
-			await this.delete(to, { recursive: true, useTrash: false });
+			await this.delete(to, { recursive: true, useTrash: false, atomic: false });
 		}
 
 		const toTargetResource = (path: string): URI => this.extUri.joinPath(to, this.extUri.relativePath(from, from.with({ path })) || '');
@@ -339,7 +339,7 @@ export class IndexedDBFileSystemProvider extends Disposable implements IFileSyst
 			await this.bulkWrite(targetFiles);
 		}
 
-		await this.delete(from, { recursive: true, useTrash: false });
+		await this.delete(from, { recursive: true, useTrash: false, atomic: false });
 	}
 
 	async delete(resource: URI, opts: IFileDeleteOptions): Promise<void> {

src/vs/platform/files/common/diskFileSystemProviderClient.ts

@@ -53,6 +53,8 @@ export class DiskFileSystemProviderClient extends Disposable implements
 				FileSystemProviderCapabilities.FileFolderCopy |
 				FileSystemProviderCapabilities.FileWriteUnlock |
 				FileSystemProviderCapabilities.FileAtomicRead |
+				FileSystemProviderCapabilities.FileAtomicWrite |
+				FileSystemProviderCapabilities.FileAtomicDelete |
 				FileSystemProviderCapabilities.FileClone;
 
 			if (this.extraCapabilities.pathCaseSensitive) {

src/vs/platform/files/common/fileService.ts

@@ -14,7 +14,7 @@ import { Disposable, DisposableStore, dispose, IDisposable, toDisposable } from
 import { TernarySearchTree } from 'vs/base/common/ternarySearchTree';
 import { Schemas } from 'vs/base/common/network';
 import { mark } from 'vs/base/common/performance';
-import { extUri, extUriIgnorePathCase, IExtUri, isAbsolutePath } from 'vs/base/common/resources';
+import { basename, dirname, extUri, extUriIgnorePathCase, IExtUri, isAbsolutePath, joinPath } from 'vs/base/common/resources';
 import { consumeStream, isReadableBufferedStream, isReadableStream, listenStream, newWriteableStream, peekReadable, peekStream, transform } from 'vs/base/common/stream';
 import { URI } from 'vs/base/common/uri';
 import { localize } from 'vs/nls';
@@ -396,7 +396,17 @@ export class FileService extends Disposable implements IFileService {
 
 			// write file: buffered
 			else {
-				await this.doWriteBuffered(provider, resource, options, bufferOrReadableOrStreamOrBufferedStream instanceof VSBuffer ? bufferToReadable(bufferOrReadableOrStreamOrBufferedStream) : bufferOrReadableOrStreamOrBufferedStream);
+				const contents = bufferOrReadableOrStreamOrBufferedStream instanceof VSBuffer ? bufferToReadable(bufferOrReadableOrStreamOrBufferedStream) : bufferOrReadableOrStreamOrBufferedStream;
+
+				// atomic write
+				if (options?.atomic !== false && options?.atomic?.postfix) {
+					await this.doWriteBufferedAtomic(provider, resource, joinPath(dirname(resource), `${basename(resource)}${options.atomic.postfix}`), options, contents);
+				}
+
+				// non-atomic write
+				else {
+					await this.doWriteBuffered(provider, resource, options, contents);
+				}
 			}
 
 			// events
@@ -416,6 +426,18 @@ export class FileService extends Disposable implements IFileService {
 			throw new Error(localize('writeFailedUnlockUnsupported', "Unable to unlock file '{0}' because provider does not support it.", this.resourceForError(resource)));
 		}
 
+		// Validate atomic support
+		const atomic = !!options?.atomic;
+		if (atomic) {
+			if (!(provider.capabilities & FileSystemProviderCapabilities.FileAtomicWrite)) {
+				throw new Error(localize('writeFailedAtomicUnsupported', "Unable to atomically write file '{0}' because provider does not support it.", this.resourceForError(resource)));
+			}
+
+			if (unlock) {
+				throw new Error(localize('writeFailedAtomicUnlock', "Unable to unlock file '{0}' because atomic write is enabled.", this.resourceForError(resource)));
+			}
+		}
+
 		// Validate via file stat meta data
 		let stat: IStat | undefined = undefined;
 		try {
@@ -579,7 +601,7 @@ export class FileService extends Disposable implements IFileService {
 		}
 
 		if (error instanceof TooLargeFileOperationError) {
-			return new TooLargeFileOperationError(message, error.fileOperationResult, error.size, error.options);
+			return new TooLargeFileOperationError(message, error.fileOperationResult, error.size, error.options as IReadFileOptions);
 		}
 
 		return new FileOperationError(message, toFileOperationResult(error), options);
@@ -959,6 +981,16 @@ export class FileService extends Disposable implements IFileService {
 			throw new Error(localize('deleteFailedTrashUnsupported', "Unable to delete file '{0}' via trash because provider does not support it.", this.resourceForError(resource)));
 		}
 
+		// Validate atomic support
+		const atomic = options?.atomic;
+		if (atomic && !(provider.capabilities & FileSystemProviderCapabilities.FileAtomicDelete)) {
+			throw new Error(localize('deleteFailedAtomicUnsupported', "Unable to delete file '{0}' atomically because provider does not support it.", this.resourceForError(resource)));
+		}
+
+		if (useTrash && atomic) {
+			throw new Error(localize('deleteFailedTrashAndAtomicUnsupported', "Unable to atomically delete file '{0}' because using trash is enabled.", this.resourceForError(resource)));
+		}
+
 		// Validate delete
 		let stat: IStat | undefined = undefined;
 		try {
@@ -990,9 +1022,10 @@ export class FileService extends Disposable implements IFileService {
 
 		const useTrash = !!options?.useTrash;
 		const recursive = !!options?.recursive;
+		const atomic = options?.atomic ?? false;
 
 		// Delete through provider
-		await provider.delete(resource, { recursive, useTrash });
+		await provider.delete(resource, { recursive, useTrash, atomic });
 
 		// Events
 		this._onDidRunOperation.fire(new FileOperationEvent(resource, FileOperation.DELETE));
@@ -1122,6 +1155,28 @@ export class FileService extends Disposable implements IFileService {
 
 	private readonly writeQueue = this._register(new ResourceQueue());
 
+	private async doWriteBufferedAtomic(provider: IFileSystemProviderWithOpenReadWriteCloseCapability, resource: URI, tempResource: URI, options: IWriteFileOptions | undefined, readableOrStreamOrBufferedStream: VSBufferReadable | VSBufferReadableStream | VSBufferReadableBufferedStream): Promise<void> {
+
+		// Write to temp resource first
+		await this.doWriteBuffered(provider, tempResource, options, readableOrStreamOrBufferedStream);
+
+		try {
+
+			// Rename over existing to ensure atomic replace
+			await provider.rename(tempResource, resource, { overwrite: true });
+		} catch (error) {
+
+			// Cleanup in case of rename error
+			try {
+				await provider.delete(tempResource, { recursive: false, useTrash: false, atomic: false });
+			} catch (error) {
+				// ignore - we want the outer error to bubble up
+			}
+
+			throw error;
+		}
+	}
+
 	private async doWriteBuffered(provider: IFileSystemProviderWithOpenReadWriteCloseCapability, resource: URI, options: IWriteFileOptions | undefined, readableOrStreamOrBufferedStream: VSBufferReadable | VSBufferReadableStream | VSBufferReadableBufferedStream): Promise<void> {
 		return this.writeQueue.queueFor(resource, this.getExtUri(provider).providerExtUri).queue(async () => {
 
@@ -1237,7 +1292,7 @@ export class FileService extends Disposable implements IFileService {
 		}
 
 		// Write through the provider
-		await provider.writeFile(resource, buffer.buffer, { create: true, overwrite: true, unlock: options?.unlock ?? false });
+		await provider.writeFile(resource, buffer.buffer, { create: true, overwrite: true, unlock: options?.unlock ?? false, atomic: options?.atomic ?? false });
 	}
 
 	private async doPipeBuffered(sourceProvider: IFileSystemProviderWithOpenReadWriteCloseCapability, source: URI, targetProvider: IFileSystemProviderWithOpenReadWriteCloseCapability, target: URI): Promise<void> {
@@ -1291,7 +1346,7 @@ export class FileService extends Disposable implements IFileService {
 	}
 
 	private async doPipeUnbufferedQueued(sourceProvider: IFileSystemProviderWithFileReadWriteCapability, source: URI, targetProvider: IFileSystemProviderWithFileReadWriteCapability, target: URI): Promise<void> {
-		return targetProvider.writeFile(target, await sourceProvider.readFile(source), { create: true, overwrite: true, unlock: false });
+		return targetProvider.writeFile(target, await sourceProvider.readFile(source), { create: true, overwrite: true, unlock: false, atomic: false });
 	}
 
 	private async doPipeUnbufferedToBuffered(sourceProvider: IFileSystemProviderWithFileReadWriteCapability, source: URI, targetProvider: IFileSystemProviderWithOpenReadWriteCloseCapability, target: URI): Promise<void> {

src/vs/platform/files/common/files.ts

@@ -283,7 +283,44 @@ export interface IFileAtomicReadOptions {
 	 * to from a different process. If you need such atomic
 	 * operations, you better use a real database as storage.
 	 */
-	readonly atomic: true;
+	readonly atomic: boolean;
+}
+
+export interface IFileAtomicOptions {
+
+	/**
+	 * The postfix is used to create a temporary file based
+	 * on the original resource. The resulting temporary
+	 * file will be in the same folder as the resource and
+	 * have `postfix` appended to the resource name.
+	 *
+	 * Example: given a file resource `file:///some/path/foo.txt`
+	 * and a postfix `.vsctmp`, the temporary file will be
+	 * created as `file:///some/path/foo.txt.vsctmp`.
+	 */
+	readonly postfix: string;
+}
+
+export interface IFileAtomicWriteOptions {
+
+	/**
+	 * The optional `atomic` flag can be used to make sure
+	 * the `writeFile` method updates the target file atomically
+	 * by first writing to a temporary file in the same folder
+	 * and then renaming it over the target.
+	 */
+	readonly atomic: IFileAtomicOptions | false;
+}
+
+export interface IFileAtomicDeleteOptions {
+
+	/**
+	 * The optional `atomic` flag can be used to make sure
+	 * the `delete` method deletes the target atomically by
+	 * first renaming it to a temporary resource in the same
+	 * folder and then deleting it.
+	 */
+	readonly atomic: IFileAtomicOptions | false;
 }
 
 export interface IFileReadLimits {
@@ -316,7 +353,7 @@ export interface IFileReadStreamOptions {
 	readonly limits?: IFileReadLimits;
 }
 
-export interface IFileWriteOptions extends IFileOverwriteOptions, IFileUnlockOptions {
+export interface IFileWriteOptions extends IFileOverwriteOptions, IFileUnlockOptions, IFileAtomicWriteOptions {
 
 	/**
 	 * Set to `true` to create a file when it does not exist. Will
@@ -358,10 +395,21 @@ export interface IFileDeleteOptions {
 
 	/**
 	 * Set to `true` to attempt to move the file to trash
-	 * instead of deleting it permanently from disk. This
-	 * option maybe not be supported on all providers.
+	 * instead of deleting it permanently from disk.
+	 *
+	 * This option maybe not be supported on all providers.
 	 */
 	readonly useTrash: boolean;
+
+	/**
+	 * The optional `atomic` flag can be used to make sure
+	 * the `delete` method deletes the target atomically by
+	 * first renaming it to a temporary resource in the same
+	 * folder and then deleting it.
+	 *
+	 * This option maybe not be supported on all providers.
+	 */
+	readonly atomic: IFileAtomicOptions | false;
 }
 
 export enum FileType {
@@ -515,10 +563,21 @@ export const enum FileSystemProviderCapabilities {
 	 */
 	FileAtomicRead = 1 << 14,
 
+	/**
+	 * Provider support to write files atomically. This implies the
+	 * provider provides the `FileReadWrite` capability too.
+	 */
+	FileAtomicWrite = 1 << 15,
+
+	/**
+	 * Provider support to delete atomically.
+	 */
+	FileAtomicDelete = 1 << 16,
+
 	/**
 	 * Provider support to clone files atomically.
 	 */
-	FileClone = 1 << 15
+	FileClone = 1 << 17
 }
 
 export interface IFileSystemProvider {
@@ -607,6 +666,26 @@ export function hasFileAtomicReadCapability(provider: IFileSystemProvider): prov
 	return !!(provider.capabilities & FileSystemProviderCapabilities.FileAtomicRead);
 }
 
+export interface IFileSystemProviderWithFileAtomicWriteCapability extends IFileSystemProvider {
+	writeFile(resource: URI, contents: Uint8Array, opts?: IFileAtomicWriteOptions): Promise<void>;
+}
+
+export function hasFileAtomicWriteCapability(provider: IFileSystemProvider): provider is IFileSystemProviderWithFileAtomicWriteCapability {
+	if (!hasReadWriteCapability(provider)) {
+		return false; // we require the `FileReadWrite` capability too
+	}
+
+	return !!(provider.capabilities & FileSystemProviderCapabilities.FileAtomicWrite);
+}
+
+export interface IFileSystemProviderWithFileAtomicDeleteCapability extends IFileSystemProvider {
+	delete(resource: URI, opts: IFileAtomicDeleteOptions): Promise<void>;
+}
+
+export function hasFileAtomicDeleteCapability(provider: IFileSystemProvider): provider is IFileSystemProviderWithFileAtomicDeleteCapability {
+	return !!(provider.capabilities & FileSystemProviderCapabilities.FileAtomicDelete);
+}
+
 export enum FileSystemProviderErrorCode {
 	FileExists = 'EntryExists',
 	FileNotFound = 'EntryNotFound',
@@ -1146,6 +1225,14 @@ export interface IWriteFileOptions {
 	 * Whether to attempt to unlock a file before writing.
 	 */
 	readonly unlock?: boolean;
+
+	/**
+	 * The optional `atomic` flag can be used to make sure
+	 * the `writeFile` method updates the target file atomically
+	 * by first writing to a temporary file in the same folder
+	 * and then renaming it over the target.
+	 */
+	readonly atomic?: IFileAtomicOptions | false;
 }
 
 export interface IResolveFileOptions {
@@ -1185,7 +1272,7 @@ export class FileOperationError extends Error {
 	constructor(
 		message: string,
 		readonly fileOperationResult: FileOperationResult,
-		readonly options?: IReadFileOptions & IWriteFileOptions & ICreateFileOptions
+		readonly options?: IReadFileOptions | IWriteFileOptions | ICreateFileOptions
 	) {
 		super(message);
 	}