Skip to content

Commit

Permalink
Merge pull request #20 from bitmeal/fsync-fix
Browse files Browse the repository at this point in the history
storage.js: check fsync capability from return code rather than using process.platform heuristics
  • Loading branch information
tex0l authored Jan 18, 2022
2 parents 59b8501 + fde8d3f commit dc851b5
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
20 changes: 15 additions & 5 deletions lib/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,25 @@ storage.flushToStorage = (options, callback) => {
flags = options.isDir ? 'r' : 'r+'
}

// Windows can't fsync (FlushFileBuffers) directories. We can live with this as it cannot cause 100% dataloss
// except in the very rare event of the first time database is loaded and a crash happens
if (flags === 'r' && (process.platform === 'win32' || process.platform === 'win64')) return callback(null)
/**
* Some OSes and/or storage backends (augmented node fs) do not support fsync (FlushFileBuffers) directories,
* or calling open() on directories at all. Flushing fails silently in this case, supported by following heuristics:
* + isDir === true
* |-- open(<dir>) -> (err.code === 'EISDIR'): can't call open() on directories (eg. BrowserFS)
* `-- fsync(<dir>) -> (errFS.code === 'EPERM' || errFS.code === 'EISDIR'): can't fsync directory: permissions are checked
* on open(); EPERM error should only occur on fsync incapability and not for general lack of permissions (e.g. Windows)
*
* We can live with this as it cannot cause 100% dataloss except in the very rare event of the first time
* database is loaded and a crash happens.
*/

fs.open(filename, flags, (err, fd) => {
if (err) return callback(err)
if (err) {
return callback((err.code === 'EISDIR' && options.isDir) ? null : err)
}
fs.fsync(fd, errFS => {
fs.close(fd, errC => {
if (errFS || errC) {
if ((errFS || errC) && !((errFS.code === 'EPERM' || errFS.code === 'EISDIR') && options.isDir)) {
const e = new Error('Failed to flush to storage')
e.errorOnFsync = errFS
e.errorOnClose = errC
Expand Down
2 changes: 1 addition & 1 deletion test/byline.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('byline', function () {
lineStream.pipe(output)
output.on('close', function () {
const out = fs.readFileSync(localPath('test.txt'), 'utf8')
const in_ = fs.readFileSync(localPath('empty.txt'), 'utf8').replace(/\n/g, '')
const in_ = fs.readFileSync(localPath('empty.txt'), 'utf8').replace(/\r?\n/g, '')
assert.equal(in_, out)
fs.unlinkSync(localPath('test.txt'))
done()
Expand Down
1 change: 1 addition & 0 deletions test_lac/openFdsLaunch.sh
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
#!/bin/sh
ulimit -n 128
node ./test_lac/openFds.test.js

0 comments on commit dc851b5

Please sign in to comment.