mirror of
https://github.com/withastro/astro.git
synced 2024-12-16 21:46:22 -05:00
fix: use atomic writes for data store file operations (#12715)
* fix: use atomic writes for data store file operations * Put tmp alongside the target * Implement locking * Refactor * Wording
This commit is contained in:
parent
799c8676df
commit
029661daa9
2 changed files with 48 additions and 5 deletions
5
.changeset/tame-bags-remember.md
Normal file
5
.changeset/tame-bags-remember.md
Normal file
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
'astro': patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fixes a bug that caused errors in dev when editing sites with large numbers of MDX pages
|
|
@ -9,6 +9,8 @@ import { contentModuleToId } from './utils.js';
|
||||||
|
|
||||||
const SAVE_DEBOUNCE_MS = 500;
|
const SAVE_DEBOUNCE_MS = 500;
|
||||||
|
|
||||||
|
const MAX_DEPTH = 10;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Extends the DataStore with the ability to change entries and write them to disk.
|
* Extends the DataStore with the ability to change entries and write them to disk.
|
||||||
* This is kept as a separate class to avoid needing node builtins at runtime, when read-only access is all that is needed.
|
* This is kept as a separate class to avoid needing node builtins at runtime, when read-only access is all that is needed.
|
||||||
|
@ -86,7 +88,7 @@ export class MutableDataStore extends ImmutableDataStore {
|
||||||
|
|
||||||
if (this.#assetImports.size === 0) {
|
if (this.#assetImports.size === 0) {
|
||||||
try {
|
try {
|
||||||
await fs.writeFile(filePath, 'export default new Map();');
|
await this.#writeFileAtomic(filePath, 'export default new Map();');
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
|
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
|
||||||
}
|
}
|
||||||
|
@ -110,7 +112,7 @@ ${imports.join('\n')}
|
||||||
export default new Map([${exports.join(', ')}]);
|
export default new Map([${exports.join(', ')}]);
|
||||||
`;
|
`;
|
||||||
try {
|
try {
|
||||||
await fs.writeFile(filePath, code);
|
await this.#writeFileAtomic(filePath, code);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
|
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
|
||||||
}
|
}
|
||||||
|
@ -122,7 +124,7 @@ export default new Map([${exports.join(', ')}]);
|
||||||
|
|
||||||
if (this.#moduleImports.size === 0) {
|
if (this.#moduleImports.size === 0) {
|
||||||
try {
|
try {
|
||||||
await fs.writeFile(filePath, 'export default new Map();');
|
await this.#writeFileAtomic(filePath, 'export default new Map();');
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
|
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
|
||||||
}
|
}
|
||||||
|
@ -143,7 +145,7 @@ export default new Map([${exports.join(', ')}]);
|
||||||
export default new Map([\n${lines.join(',\n')}]);
|
export default new Map([\n${lines.join(',\n')}]);
|
||||||
`;
|
`;
|
||||||
try {
|
try {
|
||||||
await fs.writeFile(filePath, code);
|
await this.#writeFileAtomic(filePath, code);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
|
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
|
||||||
}
|
}
|
||||||
|
@ -190,6 +192,42 @@ export default new Map([\n${lines.join(',\n')}]);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#writing = new Set<string>();
|
||||||
|
#pending = new Set<string>();
|
||||||
|
|
||||||
|
async #writeFileAtomic(filePath: PathLike, data: string, depth = 0) {
|
||||||
|
if(depth > MAX_DEPTH) {
|
||||||
|
// If we hit the max depth, we skip a write to prevent the stack from growing too large
|
||||||
|
// In theory this means we may miss the latest data, but in practice this will only happen when the file is being written to very frequently
|
||||||
|
// so it will be saved on the next write. This is unlikely to ever happen in practice, as the writes are debounced. It requires lots of writes to very large files.
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const fileKey = filePath.toString();
|
||||||
|
// If we are already writing this file, instead of writing now, flag it as pending and write it when we're done.
|
||||||
|
if (this.#writing.has(fileKey)) {
|
||||||
|
this.#pending.add(fileKey);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Prevent concurrent writes to this file by flagging it as being written
|
||||||
|
this.#writing.add(fileKey);
|
||||||
|
|
||||||
|
const tempFile = filePath instanceof URL ? new URL(`${filePath.href}.tmp`) : `${filePath}.tmp`;
|
||||||
|
try {
|
||||||
|
// Write it to a temporary file first and then move it to prevent partial reads.
|
||||||
|
await fs.writeFile(tempFile, data);
|
||||||
|
await fs.rename(tempFile, filePath);
|
||||||
|
} finally {
|
||||||
|
// We're done writing. Unflag the file and check if there are any pending writes for this file.
|
||||||
|
this.#writing.delete(fileKey);
|
||||||
|
// If there are pending writes, we need to write again to ensure we flush the latest data.
|
||||||
|
if (this.#pending.has(fileKey)) {
|
||||||
|
this.#pending.delete(fileKey);
|
||||||
|
// Call ourself recursively to write the file again
|
||||||
|
await this.#writeFileAtomic(filePath, data, depth + 1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
scopedStore(collectionName: string): DataStore {
|
scopedStore(collectionName: string): DataStore {
|
||||||
return {
|
return {
|
||||||
get: <TData extends Record<string, unknown> = Record<string, unknown>>(key: string) =>
|
get: <TData extends Record<string, unknown> = Record<string, unknown>>(key: string) =>
|
||||||
|
@ -298,7 +336,7 @@ export default new Map([\n${lines.join(',\n')}]);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
await fs.writeFile(filePath, this.toString());
|
await this.#writeFileAtomic(filePath, this.toString());
|
||||||
this.#file = filePath;
|
this.#file = filePath;
|
||||||
this.#dirty = false;
|
this.#dirty = false;
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
|
Loading…
Reference in a new issue