Skip to content

Commit

Permalink
fix: use atomic writes for data store file operations (#12715)
Browse files Browse the repository at this point in the history
* fix: use atomic writes for data store file operations

* Put tmp alongside the target

* Implement locking

* Refactor

* Wording
  • Loading branch information
ascorbic authored Dec 13, 2024
1 parent 799c867 commit 029661d
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/tame-bags-remember.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes a bug that caused errors in dev when editing sites with large numbers of MDX pages
48 changes: 43 additions & 5 deletions packages/astro/src/content/mutable-data-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { contentModuleToId } from './utils.js';

const SAVE_DEBOUNCE_MS = 500;

const MAX_DEPTH = 10;

/**
* 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.
Expand Down Expand Up @@ -86,7 +88,7 @@ export class MutableDataStore extends ImmutableDataStore {

if (this.#assetImports.size === 0) {
try {
await fs.writeFile(filePath, 'export default new Map();');
await this.#writeFileAtomic(filePath, 'export default new Map();');
} catch (err) {
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
}
Expand All @@ -110,7 +112,7 @@ ${imports.join('\n')}
export default new Map([${exports.join(', ')}]);
`;
try {
await fs.writeFile(filePath, code);
await this.#writeFileAtomic(filePath, code);
} catch (err) {
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
}
Expand All @@ -122,7 +124,7 @@ export default new Map([${exports.join(', ')}]);

if (this.#moduleImports.size === 0) {
try {
await fs.writeFile(filePath, 'export default new Map();');
await this.#writeFileAtomic(filePath, 'export default new Map();');
} catch (err) {
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
}
Expand All @@ -143,7 +145,7 @@ export default new Map([${exports.join(', ')}]);
export default new Map([\n${lines.join(',\n')}]);
`;
try {
await fs.writeFile(filePath, code);
await this.#writeFileAtomic(filePath, code);
} catch (err) {
throw new AstroError(AstroErrorData.UnknownFilesystemError, { cause: err });
}
Expand Down Expand Up @@ -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 {
return {
get: <TData extends Record<string, unknown> = Record<string, unknown>>(key: string) =>
Expand Down Expand Up @@ -298,7 +336,7 @@ export default new Map([\n${lines.join(',\n')}]);
return;
}
try {
await fs.writeFile(filePath, this.toString());
await this.#writeFileAtomic(filePath, this.toString());
this.#file = filePath;
this.#dirty = false;
} catch (err) {
Expand Down

0 comments on commit 029661d

Please sign in to comment.