diff --git a/src/vs/platform/files/node/diskFileSystemProvider.ts b/src/vs/platform/files/node/diskFileSystemProvider.ts index 8ef39ec23f0..0ab6883d353 100644 --- a/src/vs/platform/files/node/diskFileSystemProvider.ts +++ b/src/vs/platform/files/node/diskFileSystemProvider.ts @@ -148,6 +148,8 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro } } + private mapHandleToPos: Map = new Map(); + private writeHandles: Set = new Set(); private canFlush: boolean = true; @@ -187,6 +189,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 +209,10 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro async close(fd: number): Promise { 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 +233,81 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro } async read(fd: number, pos: number, data: Uint8Array, offset: number, length: number): Promise { + 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 +319,23 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro } private async doWrite(fd: number, pos: number, data: Uint8Array, offset: number, length: number): Promise { + 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 +528,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 +561,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 +569,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 +587,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 +646,4 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro dispose(this.recursiveWatcherLogLevelListener); this.recursiveWatcherLogLevelListener = undefined; } -} \ No newline at end of file +} diff --git a/src/vs/platform/files/test/node/diskFileService.test.ts b/src/vs/platform/files/test/node/diskFileService.test.ts index 6d10050ec0e..3b7c887936f 100644 --- a/src/vs/platform/files/test/node/diskFileService.test.ts +++ b/src/vs/platform/files/test/node/diskFileService.test.ts @@ -1915,4 +1915,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)); } -}); \ No newline at end of file + + 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); + }); +});