Merge from vscode 52dcb723a39ae75bee1bd56b3312d7fcdc87aeed (#6719)

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

@@ -21,7 +21,7 @@ import { Schemas } from 'vs/base/common/network';
 
 export class FileService extends Disposable implements IFileService {
 
-	_serviceBrand: ServiceIdentifier<any>;
+	_serviceBrand!: ServiceIdentifier<any>;
 
 	private readonly BUFFER_SIZE = 64 * 1024;
 
@@ -164,21 +164,25 @@ export class FileService extends Disposable implements IFileService {
 	private async doResolveFile(resource: URI, options?: IResolveFileOptions): Promise<IFileStat> {
 		const provider = await this.withProvider(resource);
 
-		// leverage a trie to check for recursive resolving
 		const resolveTo = options && options.resolveTo;
-		const trie = TernarySearchTree.forPaths<true>();
-		trie.set(resource.toString(), true);
-		if (isNonEmptyArray(resolveTo)) {
-			resolveTo.forEach(uri => trie.set(uri.toString(), true));
-		}
-
 		const resolveSingleChildDescendants = !!(options && options.resolveSingleChildDescendants);
 		const resolveMetadata = !!(options && options.resolveMetadata);
 
 		const stat = await provider.stat(resource);
 
+		let trie: TernarySearchTree<boolean> | undefined;
+
 		return this.toFileStat(provider, resource, stat, undefined, resolveMetadata, (stat, siblings) => {
 
+			// lazy trie to check for recursive resolving
+			if (!trie) {
+				trie = TernarySearchTree.forPaths<true>();
+				trie.set(resource.toString(), true);
+				if (isNonEmptyArray(resolveTo)) {
+					resolveTo.forEach(uri => trie!.set(uri.toString(), true));
+				}
+			}
+
 			// check for recursive resolving
 			if (Boolean(trie.findSuperstr(stat.resource.toString()) || trie.get(stat.resource.toString()))) {
 				return true;
@@ -253,8 +257,12 @@ export class FileService extends Disposable implements IFileService {
 	}
 
 	async exists(resource: URI): Promise<boolean> {
+		const provider = await this.withProvider(resource);
+
 		try {
-			return !!(await this.resolve(resource));
+			const stat = await provider.stat(resource);
+
+			return !!stat;
 		} catch (error) {
 			return false;
 		}

src/vs/platform/files/node/diskFileSystemProvider.ts

@@ -3,14 +3,14 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { mkdir, open, close, read, write, fdatasync, Dirent, Stats } from 'fs';
+import { mkdir, open, close, read, write, fdatasync } from 'fs';
 import { promisify } from 'util';
 import { IDisposable, Disposable, toDisposable, dispose, combinedDisposable } from 'vs/base/common/lifecycle';
 import { IFileSystemProvider, FileSystemProviderCapabilities, IFileChange, IWatchOptions, IStat, FileType, FileDeleteOptions, FileOverwriteOptions, FileWriteOptions, FileOpenOptions, FileSystemProviderErrorCode, createFileSystemProviderError, FileSystemProviderError } from 'vs/platform/files/common/files';
 import { URI } from 'vs/base/common/uri';
 import { Event, Emitter } from 'vs/base/common/event';
 import { isLinux, isWindows } from 'vs/base/common/platform';
-import { statLink, unlink, move, copy, readFile, truncate, rimraf, RimRafMode, exists, readdirWithFileTypes } from 'vs/base/node/pfs';
+import { statLink, readdir, unlink, move, copy, readFile, truncate, rimraf, RimRafMode, exists } from 'vs/base/node/pfs';
 import { normalize, basename, dirname } from 'vs/base/common/path';
 import { joinPath } from 'vs/base/common/resources';
 import { isEqual } from 'vs/base/common/extpath';
@@ -62,8 +62,15 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 		try {
 			const { stat, isSymbolicLink } = await statLink(this.toFilePath(resource)); // cannot use fs.stat() here to support links properly
 
+			let type: number;
+			if (isSymbolicLink) {
+				type = FileType.SymbolicLink | (stat.isDirectory() ? FileType.Directory : FileType.File);
+			} else {
+				type = stat.isFile() ? FileType.File : stat.isDirectory() ? FileType.Directory : FileType.Unknown;
+			}
+
 			return {
-				type: this.toType(stat, isSymbolicLink),
+				type,
 				ctime: stat.ctime.getTime(),
 				mtime: stat.mtime.getTime(),
 				size: stat.size
@@ -75,19 +82,13 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 
 	async readdir(resource: URI): Promise<[string, FileType][]> {
 		try {
-			const children = await readdirWithFileTypes(this.toFilePath(resource));
+			const children = await readdir(this.toFilePath(resource));
 
 			const result: [string, FileType][] = [];
 			await Promise.all(children.map(async child => {
 				try {
-					let type: FileType;
-					if (child.isSymbolicLink()) {
-						type = (await this.stat(joinPath(resource, child.name))).type; // always resolve target the link points to if any
-					} else {
-						type = this.toType(child);
-					}
-
-					result.push([child.name, type]);
+					const stat = await this.stat(joinPath(resource, child));
+					result.push([child, stat.type]);
 				} catch (error) {
 					this.logService.trace(error); // ignore errors for individual entries that can arise from permission denied
 				}
@@ -99,14 +100,6 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 		}
 	}
 
-	private toType(entry: Stats | Dirent, isSymbolicLink = entry.isSymbolicLink()): FileType {
-		if (isSymbolicLink) {
-			return FileType.SymbolicLink | (entry.isDirectory() ? FileType.Directory : FileType.File);
-		}
-
-		return entry.isFile() ? FileType.File : entry.isDirectory() ? FileType.Directory : FileType.Unknown;
-	}
-
 	//#endregion
 
 	//#region File Reading/Writing
@@ -148,6 +141,8 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 		}
 	}
 
+	private mapHandleToPos: Map<number, number> = new Map();
+
 	private writeHandles: Set<number> = new Set();
 	private canFlush: boolean = true;
 
@@ -187,6 +182,13 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 
 			const handle = await promisify(open)(filePath, flags);
 
+			// remember this handle to track file position of the handle
+			// we init the position to 0 since the file descriptor was
+			// just created and the position was not moved so far (see
+			// also http://man7.org/linux/man-pages/man2/open.2.html -
+			// "The file offset is set to the beginning of the file.")
+			this.mapHandleToPos.set(handle, 0);
+
 			// remember that this handle was used for writing
 			if (opts.create) {
 				this.writeHandles.add(handle);
@@ -200,6 +202,10 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 
 	async close(fd: number): Promise<void> {
 		try {
+
+			// remove this handle from map of positions
+			this.mapHandleToPos.delete(fd);
+
 			// if a handle is closed that was used for writing, ensure
 			// to flush the contents to disk if possible.
 			if (this.writeHandles.delete(fd) && this.canFlush) {
@@ -220,15 +226,81 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 	}
 
 	async read(fd: number, pos: number, data: Uint8Array, offset: number, length: number): Promise<number> {
+		const normalizedPos = this.normalizePos(fd, pos);
+
+		let bytesRead: number | null = null;
 		try {
-			const result = await promisify(read)(fd, data, offset, length, pos);
+			const result = await promisify(read)(fd, data, offset, length, normalizedPos);
+
 			if (typeof result === 'number') {
-				return result; // node.d.ts fail
+				bytesRead = result; // node.d.ts fail
+			} else {
+				bytesRead = result.bytesRead;
 			}
 
-			return result.bytesRead;
+			return bytesRead;
 		} catch (error) {
 			throw this.toFileSystemProviderError(error);
+		} finally {
+			this.updatePos(fd, normalizedPos, bytesRead);
+		}
+	}
+
+	private normalizePos(fd: number, pos: number): number | null {
+
+		// when calling fs.read/write we try to avoid passing in the "pos" argument and
+		// rather prefer to pass in "null" because this avoids an extra seek(pos)
+		// call that in some cases can even fail (e.g. when opening a file over FTP -
+		// see https://github.com/microsoft/vscode/issues/73884).
+		//
+		// as such, we compare the passed in position argument with our last known
+		// position for the file descriptor and use "null" if they match.
+		if (pos === this.mapHandleToPos.get(fd)) {
+			return null;
+		}
+
+		return pos;
+	}
+
+	private updatePos(fd: number, pos: number | null, bytesLength: number | null): void {
+		const lastKnownPos = this.mapHandleToPos.get(fd);
+		if (typeof lastKnownPos === 'number') {
+
+			// pos !== null signals that previously a position was used that is
+			// not null. node.js documentation explains, that in this case
+			// the internal file pointer is not moving and as such we do not move
+			// our position pointer.
+			//
+			// Docs: "If position is null, data will be read from the current file position,
+			// and the file position will be updated. If position is an integer, the file position
+			// will remain unchanged."
+			if (typeof pos === 'number') {
+				// do not modify the position
+			}
+
+			// bytesLength = number is a signal that the read/write operation was
+			// successful and as such we need to advance the position in the Map
+			//
+			// Docs (http://man7.org/linux/man-pages/man2/read.2.html):
+			// "On files that support seeking, the read operation commences at the
+			// file offset, and the file offset is incremented by the number of
+			// bytes read."
+			//
+			// Docs (http://man7.org/linux/man-pages/man2/write.2.html):
+			// "For a seekable file (i.e., one to which lseek(2) may be applied, for
+			// example, a regular file) writing takes place at the file offset, and
+			// the file offset is incremented by the number of bytes actually
+			// written."
+			else if (typeof bytesLength === 'number') {
+				this.mapHandleToPos.set(fd, lastKnownPos + bytesLength);
+			}
+
+			// bytesLength = null signals an error in the read/write operation
+			// and as such we drop the handle from the Map because the position
+			// is unspecificed at this point.
+			else {
+				this.mapHandleToPos.delete(fd);
+			}
 		}
 	}
 
@@ -240,15 +312,23 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 	}
 
 	private async doWrite(fd: number, pos: number, data: Uint8Array, offset: number, length: number): Promise<number> {
+		const normalizedPos = this.normalizePos(fd, pos);
+
+		let bytesWritten: number | null = null;
 		try {
-			const result = await promisify(write)(fd, data, offset, length, pos);
+			const result = await promisify(write)(fd, data, offset, length, normalizedPos);
+
 			if (typeof result === 'number') {
-				return result; // node.d.ts fail
+				bytesWritten = result; // node.d.ts fail
+			} else {
+				bytesWritten = result.bytesWritten;
 			}
 
-			return result.bytesWritten;
+			return bytesWritten;
 		} catch (error) {
 			throw this.toFileSystemProviderError(error);
+		} finally {
+			this.updatePos(fd, normalizedPos, bytesWritten);
 		}
 	}
 
@@ -441,14 +521,17 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 						watcherOptions?: IWatcherOptions
 					): WindowsWatcherService | UnixWatcherService | NsfwWatcherService
 				};
-				let watcherOptions = undefined;
 
+				let watcherOptions: IWatcherOptions | undefined = undefined;
+
+				// requires a polling watcher
 				if (this.watcherOptions && this.watcherOptions.usePolling) {
-					// requires a polling watcher
 					watcherImpl = UnixWatcherService;
 					watcherOptions = this.watcherOptions;
-				} else {
-					// Single Folder Watcher
+				}
+
+				// Single Folder Watcher
+				else {
 					if (this.recursiveFoldersToWatch.length === 1) {
 						if (isWindows) {
 							watcherImpl = WindowsWatcherService;
@@ -471,6 +554,7 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 						if (msg.type === 'error') {
 							this._onDidWatchErrorOccur.fire(msg.message);
 						}
+
 						this.logService[msg.type](msg.message);
 					},
 					this.logService.getLevel() === LogLevel.Trace,
@@ -478,7 +562,7 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 				);
 
 				if (!this.recursiveWatcherLogLevelListener) {
-					this.recursiveWatcherLogLevelListener = this.logService.onDidChangeLogLevel(_ => {
+					this.recursiveWatcherLogLevelListener = this.logService.onDidChangeLogLevel(() => {
 						if (this.recursiveWatcher) {
 							this.recursiveWatcher.setVerboseLogging(this.logService.getLevel() === LogLevel.Trace);
 						}
@@ -496,11 +580,13 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 				if (msg.type === 'error') {
 					this._onDidWatchErrorOccur.fire(msg.message);
 				}
+
 				this.logService[msg.type](msg.message);
 			},
 			this.logService.getLevel() === LogLevel.Trace
 		);
-		const logLevelListener = this.logService.onDidChangeLogLevel(_ => {
+
+		const logLevelListener = this.logService.onDidChangeLogLevel(() => {
 			watcherService.setVerboseLogging(this.logService.getLevel() === LogLevel.Trace);
 		});
 
@@ -553,4 +639,4 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
 		dispose(this.recursiveWatcherLogLevelListener);
 		this.recursiveWatcherLogLevelListener = undefined;
 	}
-}
+}

src/vs/platform/files/node/watcher/win32/csharpWatcherService.ts

@@ -18,7 +18,7 @@ export class OutOfProcessWin32FolderWatcher {
 
 	private ignored: glob.ParsedPattern[];
 
-	private handle: cp.ChildProcess;
+	private handle: cp.ChildProcess | undefined;
 	private restartCounter: number;
 
 	constructor(

src/vs/platform/files/test/node/diskFileService.test.ts

@@ -62,9 +62,9 @@ export class TestDiskFileSystemProvider extends DiskFileSystemProvider {
 
 	totalBytesRead: number = 0;
 
-	private invalidStatSize: boolean;
+	private invalidStatSize: boolean = false;
 
-	private _testCapabilities: FileSystemProviderCapabilities;
+	private _testCapabilities!: FileSystemProviderCapabilities;
 	get capabilities(): FileSystemProviderCapabilities {
 		if (!this._testCapabilities) {
 			this._testCapabilities =
@@ -115,7 +115,7 @@ export class TestDiskFileSystemProvider extends DiskFileSystemProvider {
 	}
 }
 
-suite('Disk File Service', () => {
+suite('Disk File Service', function () {
 
 	const parentDir = getRandomTestPath(tmpdir(), 'vsctests', 'diskfileservice');
 	const testSchema = 'test';
@@ -127,6 +127,12 @@ suite('Disk File Service', () => {
 
 	const disposables = new DisposableStore();
 
+	// Given issues such as https://github.com/microsoft/vscode/issues/78602
+	// we see random test failures when accessing the native file system. To
+	// diagnose further, we retry node.js file access tests up to 3 times to
+	// rule out any random disk issue.
+	this.retries(3);
+
 	setup(async () => {
 		const logService = new NullLogService();
 
@@ -613,7 +619,7 @@ suite('Disk File Service', () => {
 		await testMoveFolderAcrossProviders();
 	});
 
-	test('move - directory - across providers (unbuffered => buffered)', async () => {
+	test('move - directory - across providers (unbuffered => buffered)', async function () {
 		setCapabilities(fileProvider, FileSystemProviderCapabilities.FileReadWrite);
 		setCapabilities(testProvider, FileSystemProviderCapabilities.FileOpenReadWriteClose);
 
@@ -1915,4 +1921,94 @@ suite('Disk File Service', () => {
 	function hasChange(changes: IFileChange[], type: FileChangeType, resource: URI): boolean {
 		return changes.some(change => change.type === type && isEqual(change.resource, resource));
 	}
-});
+
+	test('read - mixed positions', async () => {
+		const resource = URI.file(join(testDir, 'lorem.txt'));
+
+		// read multiple times from position 0
+		let buffer = VSBuffer.alloc(1024);
+		let fd = await fileProvider.open(resource, { create: false });
+		for (let i = 0; i < 3; i++) {
+			await fileProvider.read(fd, 0, buffer.buffer, 0, 26);
+			assert.equal(buffer.slice(0, 26).toString(), 'Lorem ipsum dolor sit amet');
+		}
+		await fileProvider.close(fd);
+
+		// read multiple times at various locations
+		buffer = VSBuffer.alloc(1024);
+		fd = await fileProvider.open(resource, { create: false });
+
+		let posInFile = 0;
+
+		await fileProvider.read(fd, posInFile, buffer.buffer, 0, 26);
+		assert.equal(buffer.slice(0, 26).toString(), 'Lorem ipsum dolor sit amet');
+		posInFile += 26;
+
+		await fileProvider.read(fd, posInFile, buffer.buffer, 0, 1);
+		assert.equal(buffer.slice(0, 1).toString(), ',');
+		posInFile += 1;
+
+		await fileProvider.read(fd, posInFile, buffer.buffer, 0, 12);
+		assert.equal(buffer.slice(0, 12).toString(), ' consectetur');
+		posInFile += 12;
+
+		await fileProvider.read(fd, 98 /* no longer in sequence of posInFile */, buffer.buffer, 0, 9);
+		assert.equal(buffer.slice(0, 9).toString(), 'fermentum');
+
+		await fileProvider.read(fd, 27, buffer.buffer, 0, 12);
+		assert.equal(buffer.slice(0, 12).toString(), ' consectetur');
+
+		await fileProvider.read(fd, 26, buffer.buffer, 0, 1);
+		assert.equal(buffer.slice(0, 1).toString(), ',');
+
+		await fileProvider.read(fd, 0, buffer.buffer, 0, 26);
+		assert.equal(buffer.slice(0, 26).toString(), 'Lorem ipsum dolor sit amet');
+
+		await fileProvider.read(fd, posInFile /* back in sequence */, buffer.buffer, 0, 11);
+		assert.equal(buffer.slice(0, 11).toString(), ' adipiscing');
+
+		await fileProvider.close(fd);
+	});
+
+	test('write - mixed positions', async () => {
+		const resource = URI.file(join(testDir, 'lorem.txt'));
+
+		const buffer = VSBuffer.alloc(1024);
+		const fdWrite = await fileProvider.open(resource, { create: true });
+		const fdRead = await fileProvider.open(resource, { create: false });
+
+		let posInFileWrite = 0;
+		let posInFileRead = 0;
+
+		const initialContents = VSBuffer.fromString('Lorem ipsum dolor sit amet');
+		await fileProvider.write(fdWrite, posInFileWrite, initialContents.buffer, 0, initialContents.byteLength);
+		posInFileWrite += initialContents.byteLength;
+
+		await fileProvider.read(fdRead, posInFileRead, buffer.buffer, 0, 26);
+		assert.equal(buffer.slice(0, 26).toString(), 'Lorem ipsum dolor sit amet');
+		posInFileRead += 26;
+
+		const contents = VSBuffer.fromString('Hello World');
+
+		await fileProvider.write(fdWrite, posInFileWrite, contents.buffer, 0, contents.byteLength);
+		posInFileWrite += contents.byteLength;
+
+		await fileProvider.read(fdRead, posInFileRead, buffer.buffer, 0, contents.byteLength);
+		assert.equal(buffer.slice(0, contents.byteLength).toString(), 'Hello World');
+		posInFileRead += contents.byteLength;
+
+		await fileProvider.write(fdWrite, 6, contents.buffer, 0, contents.byteLength);
+
+		await fileProvider.read(fdRead, 0, buffer.buffer, 0, 11);
+		assert.equal(buffer.slice(0, 11).toString(), 'Lorem Hello');
+
+		await fileProvider.write(fdWrite, posInFileWrite, contents.buffer, 0, contents.byteLength);
+		posInFileWrite += contents.byteLength;
+
+		await fileProvider.read(fdRead, posInFileWrite - contents.byteLength, buffer.buffer, 0, contents.byteLength);
+		assert.equal(buffer.slice(0, contents.byteLength).toString(), 'Hello World');
+
+		await fileProvider.close(fdWrite);
+		await fileProvider.close(fdRead);
+	});
+});