feat: 🎸 add locking for FSA writable handles #1229
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements exclusive file locking for File System Access (FSA) API handles, specifically for FileSystemSyncAccessHandle and FileSystemWritableFileStream. The implementation follows the FSA specification by preventing concurrent access when a file is locked, throwing NoModificationAllowedError when lock acquisition is attempted on an already-locked file.
Changes:
- Introduced
FileLockManagerclass to manage file locks using a path-to-boolean map - Integrated lock checking and acquisition/release into sync access handles and writable streams for both node-to-fsa and core FSA implementations
- Added comprehensive test coverage for locking behavior across different scenarios
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/fsa/FileLockManager.ts | New lock manager class with acquire/release/check operations |
| src/fsa/tests/FileLockManager.test.ts | Comprehensive unit tests for FileLockManager |
| src/fsa/types.ts | Added locks field to CoreFsaContext |
| src/fsa/util.ts | Added newNoModificationAllowedError helper and locks to context creation |
| src/fsa/index.ts | Exported FileLockManager class |
| src/fsa/CoreFileSystemFileHandle.ts | Added lock checks before creating sync handles and writable streams |
| src/fsa/CoreFileSystemSyncAccessHandle.ts | Acquires lock in constructor, releases in close() |
| src/fsa/CoreFileSystemWritableFileStream.ts | Acquires lock in start(), releases in close() and abort() |
| src/fsa/tests/CoreFileSystemFileHandle.test.ts | Added comprehensive locking tests |
| src/node-to-fsa/types.ts | Added locks field to NodeFsaContext |
| src/node-to-fsa/util.ts | Added newNoModificationAllowedError helper and locks to context creation |
| src/node-to-fsa/NodeFileSystemFileHandle.ts | Added lock checks before creating sync handles and writable streams |
| src/node-to-fsa/NodeFileSystemSyncAccessHandle.ts | Acquires lock in constructor, releases in close() |
| src/node-to-fsa/NodeFileSystemWritableFileStream.ts | Acquires lock in start(), releases in close() and abort() with error handling |
| src/node-to-fsa/tests/NodeFileSystemFileHandle.test.ts | Added comprehensive locking tests |
| src/node-to-fsa/tests/NodeFileSystemSyncAccessHandle.test.ts | Added locking tests for sync access handles |
| export class FileLockManager { | ||
| private locks: Map<string, boolean> = new Map(); | ||
|
|
||
| public acquireLock(path: string): boolean { | ||
| if (this.locks.get(path)) { | ||
| return false; | ||
| } | ||
| this.locks.set(path, true); | ||
| return true; | ||
| } | ||
|
|
||
| public releaseLock(path: string): void { | ||
| this.locks.delete(path); | ||
| } | ||
|
|
||
| public isLocked(path: string): boolean { | ||
| return this.locks.get(path) ?? false; | ||
| } | ||
|
|
||
| public clear(): void { | ||
| this.locks.clear(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing documentation: The FileLockManager class and its public methods lack JSDoc comments. Consider adding documentation to explain the purpose of the class, the locking mechanism, and the expected behavior of each method, especially the return value of acquireLock.
|
🎉 This PR is included in version 4.54.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #1220
Closes #1018