Skip to content

Commit

Permalink
Save: Flush to disk after writing to file (fixes #9589)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bpasero
bpasero committed Aug 9, 2016
1 parent 6dd65cc commit 974bce4
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 17 deletions.
8 changes: 5 additions & 3 deletions src/typings/node.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1442,6 +1442,8 @@ declare module "fs" {
export function futimesSync(fd: number, atime: Date, mtime: Date): void;
export function fsync(fd: number, callback?: (err?: NodeJS.ErrnoException) => void): void;
export function fsyncSync(fd: number): void;
export function fdatasync(fd: number, callback?: (err?: NodeJS.ErrnoException) => void): void;
export function fdatasyncSync(fd: number): void;
export function write(fd: number, buffer: Buffer, offset: number, length: number, position: number, callback?: (err: NodeJS.ErrnoException, written: number, buffer: Buffer) => void): void;
export function write(fd: number, buffer: Buffer, offset: number, length: number, callback?: (err: NodeJS.ErrnoException, written: number, buffer: Buffer) => void): void;
export function write(fd: number, data: any, callback?: (err: NodeJS.ErrnoException, written: number, str: string) => void): void;
Expand Down Expand Up @@ -1503,9 +1505,9 @@ declare module "fs" {
* @param options An object with optional {encoding} and {flag} properties. If {encoding} is specified, readFileSync returns a string; otherwise it returns a Buffer.
*/
export function readFileSync(filename: string, options?: { flag?: string; }): Buffer;
export function writeFile(filename: string, data: any, callback?: (err: NodeJS.ErrnoException) => void): void;
export function writeFile(filename: string, data: any, options: { encoding?: string; mode?: number; flag?: string; }, callback?: (err: NodeJS.ErrnoException) => void): void;
export function writeFile(filename: string, data: any, options: { encoding?: string; mode?: string; flag?: string; }, callback?: (err: NodeJS.ErrnoException) => void): void;
export function writeFile(filename: string|number, data: any, callback?: (err: NodeJS.ErrnoException) => void): void;
export function writeFile(filename: string|number, data: any, options: { encoding?: string; mode?: number; flag?: string; }, callback?: (err: NodeJS.ErrnoException) => void): void;
export function writeFile(filename: string|number, data: any, options: { encoding?: string; mode?: string; flag?: string; }, callback?: (err: NodeJS.ErrnoException) => void): void;
export function writeFileSync(filename: string, data: any, options?: { encoding?: string; mode?: number; flag?: string; }): void;
export function writeFileSync(filename: string, data: any, options?: { encoding?: string; mode?: string; flag?: string; }): void;
export function appendFile(filename: string, data: any, options: { encoding?: string; mode?: number; flag?: string; }, callback?: (err: NodeJS.ErrnoException) => void): void;
Expand Down
32 changes: 32 additions & 0 deletions src/vs/base/node/extfs.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,4 +312,36 @@ export function mv(source: string, target: string, callback: (error: Error) => v

return callback(err);
});
}

// Calls fs.writeFile() followed by a fs.sync() call to flush the changes to disk
// We do this in cases where we want to make sure the data is really on disk and
// not in some cache.
//
// See https://github.com/nodejs/node/blob/v5.10.0/lib/fs.js#L1194
export function writeFileAndFlush(path: string, data: string | NodeBuffer, options: { encoding?: string; mode?: number; flag?: string; }, callback: (error: Error) => void): void {
if (!options) {
options = { encoding: 'utf8', mode: 0o666, flag: 'w' };
} else if (typeof options === 'string') {
options = { encoding: <string>options, mode: 0o666, flag: 'w' };
}

// Open the file with same flags and mode as fs.writeFile()
fs.open(path, options.flag, options.mode, (openError, fd) => {
if (openError) {
return callback(openError);
}

// 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) => {
if (writeError) {
return fs.close(fd, () => callback(writeError)); // still need to close the handle on error!
}

// Flush contents (not metadata) of the file to disk
fs.fdatasync(fd, (syncError) => {
return fs.close(fd, (closeError) => callback(syncError || closeError)); // make sure to carry over the fdatasync error if any!
});
});
});
}
6 changes: 6 additions & 0 deletions src/vs/base/node/pfs.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ export function writeFile(path: string, data: any, encoding: string = 'utf8'): P
return nfcall(fs.writeFile, path, data, encoding);
}

export function writeFileAndFlush(path: string, data: string, encoding?: string): Promise;
export function writeFileAndFlush(path: string, data: NodeBuffer, encoding?: string): Promise;
export function writeFileAndFlush(path: string, data: any, encoding: string = 'utf8'): Promise {
return nfcall(extfs.writeFileAndFlush, path, data, encoding);
}

/**
* Read a dir and return only subfolders
*/
Expand Down
50 changes: 38 additions & 12 deletions src/vs/base/test/node/extfs/extfs.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import extfs = require('vs/base/node/extfs');
suite('Extfs', () => {

test('mkdirp', function (done: () => void) {
var id = uuid.generateUuid();
var parentDir = path.join(os.tmpdir(), 'vsctests', id);
var newDir = path.join(parentDir, 'extfs', id);
const id = uuid.generateUuid();
const parentDir = path.join(os.tmpdir(), 'vsctests', id);
const newDir = path.join(parentDir, 'extfs', id);

extfs.mkdirp(newDir, 493, (error) => {
assert.ok(!error);
Expand All @@ -31,12 +31,12 @@ suite('Extfs', () => {
});

test('copy, move and delete', function (done: () => void) {
var id = uuid.generateUuid();
var id2 = uuid.generateUuid();
var sourceDir = require.toUrl('./fixtures');
var parentDir = path.join(os.tmpdir(), 'vsctests', 'extfs');
var targetDir = path.join(parentDir, id);
var targetDir2 = path.join(parentDir, id2);
const id = uuid.generateUuid();
const id2 = uuid.generateUuid();
const sourceDir = require.toUrl('./fixtures');
const parentDir = path.join(os.tmpdir(), 'vsctests', 'extfs');
const targetDir = path.join(parentDir, id);
const targetDir2 = path.join(parentDir, id2);

extfs.copy(sourceDir, targetDir, (error) => {
assert.ok(!error);
Expand Down Expand Up @@ -76,9 +76,9 @@ suite('Extfs', () => {

test('readdir', function (done: () => void) {
if (strings.canNormalize && typeof process.versions['electron'] !== 'undefined' /* needs electron */) {
var id = uuid.generateUuid();
var parentDir = path.join(os.tmpdir(), 'vsctests', id);
var newDir = path.join(parentDir, 'extfs', id, 'öäü');
const id = uuid.generateUuid();
const parentDir = path.join(os.tmpdir(), 'vsctests', id);
const newDir = path.join(parentDir, 'extfs', id, 'öäü');

extfs.mkdirp(newDir, 493, (error) => {
assert.ok(!error);
Expand All @@ -94,4 +94,30 @@ suite('Extfs', () => {
done();
}
});

test('writeFileAndFlush', function (done: () => void) {
const id = uuid.generateUuid();
const parentDir = path.join(os.tmpdir(), 'vsctests', id);
const newDir = path.join(parentDir, 'extfs', id);
const testFile = path.join(newDir, 'flushed.txt');

extfs.mkdirp(newDir, 493, (error) => {
assert.ok(!error);
assert.ok(fs.existsSync(newDir));

extfs.writeFileAndFlush(testFile, 'Hello World', null, (error) => {
assert.ok(!error);
assert.equal(fs.readFileSync(testFile), 'Hello World');

const largeString = (new Array(100 * 1024)).join('Large String\n');

extfs.writeFileAndFlush(testFile, largeString, null, (error) => {
assert.ok(!error);
assert.equal(fs.readFileSync(testFile), largeString);

done();
});
});
});
});
});
4 changes: 2 additions & 2 deletions src/vs/workbench/services/files/node/fileService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,13 +272,13 @@ export class FileService implements IFileService {

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

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

// 4.) set contents
Expand Down

2 comments on commit 974bce4

@chrismaltby
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bpasero Not sure you'll see this message since the commit is 3 years old at this point but thanks so much for this! The commit ended up being really useful to fix almost the same issue occurring in my GameBoy game maker, GB Studio, which I ended up implementing here chrismaltby/gb-studio#263 Thanks!

@bpasero
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrismaltby cool :)

Please sign in to comment.