Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ npm install diskstore --save

- `new DiskStore(options)`
- {String} cacheDir - root cache dir
- {Boolean} [fallback=true] - when the internal `rename` fails with `EXDEV` (the `.tmp` dir and the target file are on different filesystems), retry by writing a temp file in the target file's own directory and rename it there, which stays atomic. Set to `false` to let the `EXDEV` error throw.
- {String} [fallbackTmpfileName] - the temp file name used by the fallback, placed in the target file's directory. Defaults to a random name. A fixed name is only safe when concurrent writes to the same directory cannot happen; otherwise keep the default so each write gets a unique temp file.
- `async get(relativePath)` read data from relativePath if the file exists.
- {String} relativePath - file path relative to root cache dir
- `async set(relativePath, data)` write data to relativePath
Expand Down
33 changes: 33 additions & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ class DiskStore extends Base {
*
* @param {Object} options
* - {String} cacheDir - cache dir
* - {Boolean} [fallback=true] - on cross-device rename (EXDEV), retry via the target's own dir
* - {String} [fallbackTmpfileName] - temp file name used by the fallback, defaults to a random
* one. A fixed name is only safe when concurrent writes to the same directory cannot happen.
*/
constructor(options = {}) {
assert(options.cacheDir, '[DiskStore] options.cacheDir is required');
Expand All @@ -22,6 +25,15 @@ class DiskStore extends Base {
return this.options.cacheDir;
}

// on by default, only off when explicitly set to false
get fallback() {
return this.options.fallback !== false;
}

get fallbackTmpfileName() {
return this.options.fallbackTmpfileName;
}

async get(relativePath) {
const filepath = path.join(this.cacheDir, relativePath);
try {
Expand All @@ -47,6 +59,27 @@ class DiskStore extends Base {
await fs.rename(tmpfile, filepath);
} catch (err) {
await fs.rm(tmpfile, { force: true });
// rename across filesystems throws EXDEV, retry via the target's own dir
if (err.code === 'EXDEV' && this.fallback) {
await this._renameWithFallback(dir, filepath, data);
return;
}
throw err;
}
}

// write a temp file in the target's own dir then rename, so it stays on one
// filesystem and keeps the atomic replace
async _renameWithFallback(dir, filepath, data) {
const fallbackTmpfile = path.join(
dir,
this.fallbackTmpfileName || `.${path.basename(filepath)}.${crypto.randomUUID()}.tmp`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Using a static, shared fallbackTmpfileName across different target files in the same directory can lead to severe race conditions and data corruption during concurrent writes. For example, if store.set('dir/fileA') and store.set('dir/fileB') run concurrently, both will attempt to write to and rename the exact same temporary file path (dir/fallbackTmpfileName), causing one to overwrite the other's data or fail with ENOENT.

To prevent this, we should ensure that the temporary file name is always unique per write operation, even when a custom name/prefix is provided. Appending a unique identifier (like a UUID) to the custom name is a robust way to avoid these conflicts.

      this.fallbackTmpfileName
        ? `${this.fallbackTmpfileName}.${crypto.randomUUID()}`
        : `.${path.basename(filepath)}.${crypto.randomUUID()}.tmp`

);
try {
await fs.writeFile(fallbackTmpfile, data);
await fs.rename(fallbackTmpfile, filepath);
} catch (err) {
await fs.rm(fallbackTmpfile, { force: true });
throw err;
}
}
Expand Down
93 changes: 93 additions & 0 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,99 @@ describe('test/index.test.js', () => {
assert.deepEqual(data, Buffer.from('a foo bar'));
});

describe('EXDEV fallback', () => {
// simulate .tmp being on a different device than the target: rename from .tmp
// throws EXDEV, everything else falls through to the real rename
function mockCrossDevice() {
const originRename = fs.rename;
mm(fs, 'rename', async (src, dest) => {
if (src.includes(`${path.sep}.tmp${path.sep}`)) {
const err = new Error('EXDEV: cross-device link not permitted');
err.code = 'EXDEV';
throw err;
}
return originRename(src, dest);
});
}

it('should fallback to same-dir write when rename is cross-device', async function() {
mockCrossDevice();
await diskStore.set('exdev/a', 'exdev value');
const data = await diskStore.get('exdev/a');
assert.deepEqual(data, Buffer.from('exdev value'));
// the fallback temp file should be cleaned up, nothing left in the dir
const files = await fs.readdir(path.join(cacheDir, 'exdev'));
assert.deepEqual(files, [ 'a' ]);
});

it('should use custom fallbackTmpfileName', async function() {
const store = new DiskStore({
cacheDir,
fallbackTmpfileName: '.custom.fallback.tmp',
});
await store.ready();
// record the source of the same-dir (fallback) rename to prove the custom name is used
const renamed = [];
const originRename = fs.rename;
mm(fs, 'rename', async (src, dest) => {
if (src.includes(`${path.sep}.tmp${path.sep}`)) {
const err = new Error('EXDEV: cross-device link not permitted');
err.code = 'EXDEV';
throw err;
}
renamed.push(src);
return originRename(src, dest);
});
await store.set('exdev-custom', 'v');
const data = await store.get('exdev-custom');
assert.deepEqual(data, Buffer.from('v'));
assert.deepEqual(renamed, [ path.join(cacheDir, '.custom.fallback.tmp') ]);
// the fallback temp file is removed once used
assert(await store.get('.custom.fallback.tmp') === null);
});

it('should clean up and throw when the fallback rename also fails', async function() {
mm(fs, 'rename', async src => {
if (src.includes(`${path.sep}.tmp${path.sep}`)) {
const err = new Error('EXDEV: cross-device link not permitted');
err.code = 'EXDEV';
throw err;
}
// the fallback (same-dir) rename fails too
throw new Error('mock fallback rename error');
});
try {
await diskStore.set('exdev-fail/a', 'v');
assert(false, 'should not run here');
} catch (err) {
assert(err.message === 'mock fallback rename error');
}
// both the primary and the fallback temp files should be cleaned up
const files = await fs.readdir(path.join(cacheDir, 'exdev-fail'));
assert(files.length === 0);
const tmpFiles = await fs.readdir(path.join(cacheDir, '.tmp'));
assert(tmpFiles.length === 0);
});

it('should throw EXDEV when fallback disabled', async function() {
const store = new DiskStore({
cacheDir,
fallback: false,
});
await store.ready();
mockCrossDevice();
try {
await store.set('exdev-off', 'v');
assert(false, 'should not run here');
} catch (err) {
assert(err.code === 'EXDEV');
}
// the primary temp file should still be cleaned up
const files = await fs.readdir(path.join(cacheDir, '.tmp'));
assert(files.length === 0);
});
});

describe('write atomic', () => {
it('should write be atomic', async function() {
await coffee.fork('write_big_file.js', [])
Expand Down
Loading