Skip to content
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

Use r+ with truncation when saving existing files #31733

Merged
merged 7 commits into from
Aug 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 3 additions & 5 deletions src/vs/base/node/extfs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,15 +332,13 @@ export function mv(source: string, target: string, callback: (error: Error) => v
//
// See https://github.com/nodejs/node/blob/v5.10.0/lib/fs.js#L1194
let canFlush = true;
export function writeFileAndFlush(path: string, data: string | NodeBuffer, options: { encoding?: string; mode?: number; flag?: string; }, callback: (error: Error) => void): void {
export function writeFileAndFlush(path: string, data: string | NodeBuffer, options: { mode?: number; flag?: string; }, callback: (error: Error) => void): void {
if (!canFlush) {
return fs.writeFile(path, data, options, callback);
}

if (!options) {
options = { encoding: 'utf8', mode: 0o666, flag: 'w' };
} else if (typeof options === 'string') {
options = { encoding: <string>options, mode: 0o666, flag: 'w' };
options = { mode: 0o666, flag: 'w' };
}

// Open the file with same flags and mode as fs.writeFile()
Expand All @@ -350,7 +348,7 @@ export function writeFileAndFlush(path: string, data: string | NodeBuffer, optio
}

// It is valid to pass a fd handle to fs.writeFile() and this will keep the handle open!
fs.writeFile(fd, data, options.encoding, (writeError) => {
fs.writeFile(fd, data, (writeError) => {
if (writeError) {
return fs.close(fd, () => callback(writeError)); // still need to close the handle on error!
}
Expand Down
12 changes: 8 additions & 4 deletions src/vs/base/node/pfs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ export function touch(path: string): TPromise<void> {
return nfcall(fs.utimes, path, now, now);
}

export function truncate(path: string, len: number): TPromise<void> {
return nfcall(fs.truncate, path, len);
}

export function readFile(path: string): TPromise<Buffer>;
export function readFile(path: string, encoding: string): TPromise<string>;
export function readFile(path: string, encoding?: string): TPromise<Buffer | string> {
Expand All @@ -120,12 +124,12 @@ export function readFile(path: string, encoding?: string): TPromise<Buffer | str
// Therefor we use a Queue on the path that is given to us to sequentialize calls to the same path properly.
const writeFilePathQueue: { [path: string]: Queue<void> } = Object.create(null);

export function writeFile(path: string, data: string, encoding?: string): TPromise<void>;
export function writeFile(path: string, data: NodeBuffer, encoding?: string): TPromise<void>;
export function writeFile(path: string, data: any, encoding: string = 'utf8'): TPromise<void> {
export function writeFile(path: string, data: string, options?: { mode?: number; flag?: string; }): TPromise<void>;
export function writeFile(path: string, data: NodeBuffer, options?: { mode?: number; flag?: string; }): TPromise<void>;
export function writeFile(path: string, data: any, options?: { mode?: number; flag?: string; }): TPromise<void> {
let queueKey = toQueueKey(path);

return ensureWriteFileQueue(queueKey).queue(() => nfcall(extfs.writeFileAndFlush, path, data, encoding));
return ensureWriteFileQueue(queueKey).queue(() => nfcall(extfs.writeFileAndFlush, path, data, options));
}

function toQueueKey(path: string): string {
Expand Down
51 changes: 34 additions & 17 deletions src/vs/workbench/services/files/node/fileService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,30 +348,47 @@ export class FileService implements IFileService {

// 3.) check to add UTF BOM
return addBomPromise.then(addBom => {
let writeFilePromise: TPromise<void>;

// Write fast if we do UTF 8 without BOM
if (!addBom && encodingToWrite === encoding.UTF8) {
writeFilePromise = pfs.writeFile(absolutePath, value, encoding.UTF8);
}

// Otherwise use encoding lib
else {
const encoded = encoding.encode(value, encodingToWrite, { addBOM: addBom });
writeFilePromise = pfs.writeFile(absolutePath, encoded);
}

// 4.) set contents
return writeFilePromise.then(() => {
// 4.) set contents and resolve
return this.doSetContentsAndResolve(resource, absolutePath, value, addBom, encodingToWrite, { mode: 0o666, flag: 'w' }).then(undefined, error => {
// Can't use 'w' for hidden files on Windows (see https://github.com/Microsoft/vscode/issues/931)
if (!exists || error.code !== 'EPERM' || !isWindows) {
return TPromise.wrapError(error);
}

// 5.) resolve
return this.resolve(resource);
// 'r+' always works for existing files, but we need to truncate manually
// 5.) truncate
return pfs.truncate(absolutePath, 0).then(() => {
// 6.) set contents and resolve again
return this.doSetContentsAndResolve(resource, absolutePath, value, addBom, encodingToWrite, { mode: 0o666, flag: 'r+' });
});
});
});
});
});
}

private doSetContentsAndResolve(resource: uri, absolutePath: string, value: string, addBOM: boolean, encodingToWrite: string, options: { mode?: number; flag?: string; }): TPromise<IFileStat> {
let writeFilePromise: TPromise<void>;

// Write fast if we do UTF 8 without BOM
if (!addBOM && encodingToWrite === encoding.UTF8) {
writeFilePromise = pfs.writeFile(absolutePath, value, options);
}

// Otherwise use encoding lib
else {
const encoded = encoding.encode(value, encodingToWrite, { addBOM });
writeFilePromise = pfs.writeFile(absolutePath, encoded, options);
}

// set contents
return writeFilePromise.then(() => {

// resolve
return this.resolve(resource);
});
}

public createFile(resource: uri, content: string = ''): TPromise<IFileStat> {

// Create file
Expand Down