refactor(storage): 重构文件存储实现以支持异步安全锁和原子写入#98
Merged
Conversation
- 替换同步锁机制为异步键锁管理器，提升并发性能 - 实现原子写入功能，通过临时文件防止写入过程中的数据损坏 - 添加资源释放接口(IDisposable)和对象销毁检查 - 集成异步IO操作，包括缓冲区大小配置选项 - 更新Godot文件存储适配器以匹配新的异步安全机制 - 优化文件读取操作，支持异步文本读取避免阻塞 - 移除旧的并发字典锁实现，统一使用新的锁管理器
审阅者指南重构 Godot 与游戏文件存储实现，使其使用共享的异步基于键的锁管理器，添加释放与对象已释放检查，引入真正的可配置缓冲的异步 I/O，并通过临时文件为游戏文件存储实现原子写入。 使用异步锁实现 FileStorage.WriteAsync 原子写入的时序图sequenceDiagram
actor GameSystem
participant FileStorage
participant IAsyncKeyLockManager
participant AsyncKeyLockScope as IAsyncDisposable
participant FileSystem
GameSystem->>FileStorage: WriteAsync(key, value)
FileStorage->>FileStorage: ObjectDisposedException.ThrowIf(_disposed, this)
FileStorage->>FileStorage: path = ToPath(key)
FileStorage->>FileStorage: tempPath = path + .tmp
FileStorage->>IAsyncKeyLockManager: AcquireLockAsync(path)
IAsyncKeyLockManager-->>FileStorage: AsyncKeyLockScope
activate FileStorage
FileStorage->>AsyncKeyLockScope: await using scope
FileStorage->>FileStorage: content = _serializer.Serialize(value)
FileStorage->>FileSystem: open FileStream(tempPath, Create, Write, None, _bufferSize, useAsync)
FileStorage->>FileSystem: create StreamWriter and WriteAsync(content)
FileStorage->>FileSystem: FlushAsync and close
FileStorage->>FileSystem: File.Move(tempPath, path, overwrite true)
AsyncKeyLockScope-->>IAsyncKeyLockManager: DisposeAsync
deactivate FileStorage
FileStorage-->>GameSystem: Task completed
note over FileStorage,FileSystem: If any exception occurs, tempPath is deleted before rethrowing
使用异步键锁的 GodotFileStorage.ExistsAsync 时序图sequenceDiagram
actor GameCode
participant GodotFileStorage
participant IAsyncKeyLockManager
participant AsyncKeyLockScope as IAsyncDisposable
participant FileSystem
participant GodotFS as Godot.FileAccess
GameCode->>GodotFileStorage: ExistsAsync(key)
GodotFileStorage->>GodotFileStorage: ObjectDisposedException.ThrowIf(_disposed, this)
GodotFileStorage->>GodotFileStorage: path = ToAbsolutePath(key)
GodotFileStorage->>IAsyncKeyLockManager: AcquireLockAsync(path)
IAsyncKeyLockManager-->>GodotFileStorage: AsyncKeyLockScope
GodotFileStorage->>AsyncKeyLockScope: await using scope
alt path is normal file path
GodotFileStorage->>FileSystem: File.Exists(path)
FileSystem-->>GodotFileStorage: exists bool
else path is Godot path
GodotFileStorage->>GodotFS: FileAccess.Open(path, Read)
GodotFS-->>GodotFileStorage: file handle or null
GodotFileStorage->>GodotFileStorage: exists = file != null
end
AsyncKeyLockScope-->>IAsyncKeyLockManager: DisposeAsync
GodotFileStorage-->>GameCode: exists bool
重构后的异步文件存储与锁的类图classDiagram
class IStorage
class IFileStorage
class ISerializer
class IAsyncKeyLockManager {
<<interface>>
AcquireLockAsync(key string) Task~IAsyncDisposable~
Dispose() void
}
class AsyncKeyLockManager {
AsyncKeyLockManager()
AcquireLockAsync(key string) Task~IAsyncDisposable~
Dispose() void
}
class GodotFileStorage {
<<sealed>>
- _lockManager IAsyncKeyLockManager
- _serializer ISerializer
- _disposed bool
+ GodotFileStorage(serializer ISerializer, lockManager IAsyncKeyLockManager)
+ Dispose() void
+ Delete(key string) void
+ DeleteAsync(key string) Task
+ Exists(key string) bool
+ ExistsAsync(key string) Task~bool~
+ Read~T~(key string) T
+ Read~T~(key string, defaultValue T) T
+ ReadAsync~T~(key string) Task~T~
+ Write~T~(key string, value T) void
+ WriteAsync~T~(key string, value T) Task
- ToAbsolutePath(key string) string
}
class FileStorage {
<<sealed>>
- _bufferSize int
- _extension string
- _lockManager IAsyncKeyLockManager
- _rootPath string
- _serializer ISerializer
- _disposed bool
+ FileStorage(rootPath string, serializer ISerializer, extension string, bufferSize int, lockManager IAsyncKeyLockManager)
+ Dispose() void
+ Delete(key string) void
+ DeleteAsync(key string) Task
+ Exists(key string) bool
+ ExistsAsync(key string) Task~bool~
+ Read~T~(key string) T
+ Read~T~(key string, defaultValue T) T
+ ReadAsync~T~(key string) Task~T~
+ Write~T~(key string, value T) void
+ WriteAsync~T~(key string, value T) Task
- ToPath(key string) string
- CleanSegment(segment string) string
}
IStorage <|.. GodotFileStorage
IDisposable <|.. GodotFileStorage
IFileStorage <|.. FileStorage
IDisposable <|.. FileStorage
AsyncKeyLockManager ..|> IAsyncKeyLockManager
GodotFileStorage --> ISerializer : uses
GodotFileStorage --> IAsyncKeyLockManager : uses
FileStorage --> ISerializer : uses
FileStorage --> IAsyncKeyLockManager : uses
跨存储实现共享异步锁管理器的架构流程图flowchart LR
subgraph Core_Assembly[GFramework.Core]
AsyncKeyLockManager
IAsyncKeyLockManager
end
subgraph Godot_Assembly[GFramework.Godot]
GodotFileStorage
end
subgraph Game_Assembly[GFramework.Game]
FileStorage
end
subgraph Core_Serializer[GFramework.Core.Serializer]
ISerializer
end
GodotFileStorage --> IAsyncKeyLockManager
FileStorage --> IAsyncKeyLockManager
AsyncKeyLockManager -.-> IAsyncKeyLockManager
GodotFileStorage --> ISerializer
FileStorage --> ISerializer
GameCode[[Game code]] --> FileStorage
GodotScene[[Godot scene scripts]] --> GodotFileStorage
FileStorage -.uses filesystem APIs.-> FileSystem[(File system)]
GodotFileStorage -.uses Godot FileAccess.-> GodotFileAccess[(Godot FileAccess)]
文件级变更
技巧与命令与 Sourcery 交互
自定义你的使用体验访问你的控制面板 来：
获取帮助Original review guide in EnglishReviewer's GuideRefactors the Godot and game file storage implementations to use a shared async key-based lock manager, add disposal and object-disposed checks, introduce truly async I/O with configurable buffering, and implement atomic writes for game file storage via temporary files. Sequence diagram for FileStorage.WriteAsync atomic write with async locksequenceDiagram
actor GameSystem
participant FileStorage
participant IAsyncKeyLockManager
participant AsyncKeyLockScope as IAsyncDisposable
participant FileSystem
GameSystem->>FileStorage: WriteAsync(key, value)
FileStorage->>FileStorage: ObjectDisposedException.ThrowIf(_disposed, this)
FileStorage->>FileStorage: path = ToPath(key)
FileStorage->>FileStorage: tempPath = path + .tmp
FileStorage->>IAsyncKeyLockManager: AcquireLockAsync(path)
IAsyncKeyLockManager-->>FileStorage: AsyncKeyLockScope
activate FileStorage
FileStorage->>AsyncKeyLockScope: await using scope
FileStorage->>FileStorage: content = _serializer.Serialize(value)
FileStorage->>FileSystem: open FileStream(tempPath, Create, Write, None, _bufferSize, useAsync)
FileStorage->>FileSystem: create StreamWriter and WriteAsync(content)
FileStorage->>FileSystem: FlushAsync and close
FileStorage->>FileSystem: File.Move(tempPath, path, overwrite true)
AsyncKeyLockScope-->>IAsyncKeyLockManager: DisposeAsync
deactivate FileStorage
FileStorage-->>GameSystem: Task completed
note over FileStorage,FileSystem: If any exception occurs, tempPath is deleted before rethrowing
Sequence diagram for GodotFileStorage.ExistsAsync with async key locksequenceDiagram
actor GameCode
participant GodotFileStorage
participant IAsyncKeyLockManager
participant AsyncKeyLockScope as IAsyncDisposable
participant FileSystem
participant GodotFS as Godot.FileAccess
GameCode->>GodotFileStorage: ExistsAsync(key)
GodotFileStorage->>GodotFileStorage: ObjectDisposedException.ThrowIf(_disposed, this)
GodotFileStorage->>GodotFileStorage: path = ToAbsolutePath(key)
GodotFileStorage->>IAsyncKeyLockManager: AcquireLockAsync(path)
IAsyncKeyLockManager-->>GodotFileStorage: AsyncKeyLockScope
GodotFileStorage->>AsyncKeyLockScope: await using scope
alt path is normal file path
GodotFileStorage->>FileSystem: File.Exists(path)
FileSystem-->>GodotFileStorage: exists bool
else path is Godot path
GodotFileStorage->>GodotFS: FileAccess.Open(path, Read)
GodotFS-->>GodotFileStorage: file handle or null
GodotFileStorage->>GodotFileStorage: exists = file != null
end
AsyncKeyLockScope-->>IAsyncKeyLockManager: DisposeAsync
GodotFileStorage-->>GameCode: exists bool
Class diagram for refactored async file storage and lockingclassDiagram
class IStorage
class IFileStorage
class ISerializer
class IAsyncKeyLockManager {
<<interface>>
AcquireLockAsync(key string) Task~IAsyncDisposable~
Dispose() void
}
class AsyncKeyLockManager {
AsyncKeyLockManager()
AcquireLockAsync(key string) Task~IAsyncDisposable~
Dispose() void
}
class GodotFileStorage {
<<sealed>>
- _lockManager IAsyncKeyLockManager
- _serializer ISerializer
- _disposed bool
+ GodotFileStorage(serializer ISerializer, lockManager IAsyncKeyLockManager)
+ Dispose() void
+ Delete(key string) void
+ DeleteAsync(key string) Task
+ Exists(key string) bool
+ ExistsAsync(key string) Task~bool~
+ Read~T~(key string) T
+ Read~T~(key string, defaultValue T) T
+ ReadAsync~T~(key string) Task~T~
+ Write~T~(key string, value T) void
+ WriteAsync~T~(key string, value T) Task
- ToAbsolutePath(key string) string
}
class FileStorage {
<<sealed>>
- _bufferSize int
- _extension string
- _lockManager IAsyncKeyLockManager
- _rootPath string
- _serializer ISerializer
- _disposed bool
+ FileStorage(rootPath string, serializer ISerializer, extension string, bufferSize int, lockManager IAsyncKeyLockManager)
+ Dispose() void
+ Delete(key string) void
+ DeleteAsync(key string) Task
+ Exists(key string) bool
+ ExistsAsync(key string) Task~bool~
+ Read~T~(key string) T
+ Read~T~(key string, defaultValue T) T
+ ReadAsync~T~(key string) Task~T~
+ Write~T~(key string, value T) void
+ WriteAsync~T~(key string, value T) Task
- ToPath(key string) string
- CleanSegment(segment string) string
}
IStorage <|.. GodotFileStorage
IDisposable <|.. GodotFileStorage
IFileStorage <|.. FileStorage
IDisposable <|.. FileStorage
AsyncKeyLockManager ..|> IAsyncKeyLockManager
GodotFileStorage --> ISerializer : uses
GodotFileStorage --> IAsyncKeyLockManager : uses
FileStorage --> ISerializer : uses
FileStorage --> IAsyncKeyLockManager : uses
Architecture flow diagram for shared async lock manager across storage implementationsflowchart LR
subgraph Core_Assembly[GFramework.Core]
AsyncKeyLockManager
IAsyncKeyLockManager
end
subgraph Godot_Assembly[GFramework.Godot]
GodotFileStorage
end
subgraph Game_Assembly[GFramework.Game]
FileStorage
end
subgraph Core_Serializer[GFramework.Core.Serializer]
ISerializer
end
GodotFileStorage --> IAsyncKeyLockManager
FileStorage --> IAsyncKeyLockManager
AsyncKeyLockManager -.-> IAsyncKeyLockManager
GodotFileStorage --> ISerializer
FileStorage --> ISerializer
GameCode[[Game code]] --> FileStorage
GodotScene[[Godot scene scripts]] --> GodotFileStorage
FileStorage -.uses filesystem APIs.-> FileSystem[(File system)]
GodotFileStorage -.uses Godot FileAccess.-> GodotFileAccess[(Godot FileAccess)]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# |  |
Mar 11, 2026 2:36p.m. | Review ↗ |
| Secrets | |
Mar 11, 2026 2:36p.m. | Review ↗ |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了两个问题，并给出了一些高层次的反馈：
- 同步方法（
Read、Write、Exists、Delete）现在通过调用它们对应的异步方法并使用GetAwaiter().GetResult()来实现，在某些环境下这会引入死锁风险或导致不必要的线程阻塞；建议要么保留一条完全同步的代码路径，要么在文档中明确说明这些 API 在有上下文绑定（比如 UI/主线程）的线程上使用是不安全的，并鼓励直接使用异步版本。 GodotFileStorage和FileStorage在任何情况下都会释放IAsyncKeyLockManager，即便它是通过依赖注入提供的；建议跟踪锁管理器是否是在内部创建的，并只在内部创建的情况下才进行释放，以避免无意中释放一个共享的或外部拥有的实例。
面向 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- 同步方法（`Read`、`Write`、`Exists`、`Delete`）现在通过调用它们对应的异步方法并使用 `GetAwaiter().GetResult()` 来实现，在某些环境下这会引入死锁风险或导致不必要的线程阻塞；建议要么保留一条完全同步的代码路径，要么在文档中明确说明这些 API 在有上下文绑定（比如 UI/主线程）的线程上使用是不安全的，并鼓励直接使用异步版本。
- `GodotFileStorage` 和 `FileStorage` 在任何情况下都会释放 `IAsyncKeyLockManager`，即便它是通过依赖注入提供的；建议跟踪锁管理器是否是在内部创建的，并只在内部创建的情况下才进行释放，以避免无意中释放一个共享的或外部拥有的实例。
## Individual Comments
### Comment 1
<location path="GFramework.Godot/Storage/GodotFileStorage.cs" line_range="53-62" />
<code_context>
- if (File.Exists(path))
- File.Delete(path);
- }
+ DeleteAsync(key).GetAwaiter().GetResult();
}
</code_context>
<issue_to_address>
**issue (bug_risk):** 在有同步上下文的环境中，对异步方法使用 GetAwaiter().GetResult() 会带来死锁风险。
这里同步方法（Delete/Exists/Read/Write）只是对其异步版本的薄包装，通过 `GetAwaiter().GetResult()` 来调用。这种模式在这些 API 被 Godot 等框架或任何会捕获同步上下文的环境中使用时尤其危险。
为减轻这种风险，可以：
1. 在所有异步方法中的 await 调用后统一加上 `ConfigureAwait(false)`，这样它们就不会尝试在原始上下文中恢复，或者
2. 让同步方法真正同步（使用同步 I/O API），并将异步方法视为单独的异步实现，而不是在异步实现之上再叠加同步封装。
即便保留当前结构，仅仅是添加 `ConfigureAwait(false)` 也会显著降低风险。
</issue_to_address>
### Comment 2
<location path="GFramework.Godot/Storage/GodotFileStorage.cs" line_range="66-69" />
<code_context>
+ ObjectDisposedException.ThrowIf(_disposed, this);
+ var path = ToPath(key);
+
+ await using (await _lockManager.AcquireLockAsync(path))
+ {
+ if (File.Exists(path))
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 建议在该类库性质的存储代码中的所有 await 操作上统一使用 ConfigureAwait(false)。
由于这一存储层可能会被 UI/游戏线程或其它上下文消费，这些 await（`AcquireLockAsync`、`File.ReadAllTextAsync`、`File.WriteAllTextAsync` 等）当前会捕获调用方的同步上下文，这可能导致死锁或上下文依赖的行为。在类库代码中，通常的做法是在这类 await 调用上使用 `ConfigureAwait(false)`，例如：
```csharp
await using (await _lockManager.AcquireLockAsync(path).ConfigureAwait(false))
{
content = await File.ReadAllTextAsync(path, Encoding.UTF8).ConfigureAwait(false);
}
```
`FileStorage` 中的其它异步方法也同样适用这个建议。
</issue_to_address>帮我变得更有用！请在每条评论上点👍或👎，我会根据你的反馈改进后续的 Review。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The synchronous methods (
Read,Write,Exists,Delete) now block on their async counterparts viaGetAwaiter().GetResult(), which can introduce deadlocks or unnecessary thread blocking in some environments; consider either preserving a purely synchronous code path or documenting that these APIs are not safe for use on a context-bound thread (e.g., UI/main thread) and encouraging direct use of the async variants. - Both
GodotFileStorageandFileStoragealways dispose theIAsyncKeyLockManager, even when it is provided via dependency injection; consider tracking whether the lock manager was created internally and only disposing it in that case, to avoid unintentionally disposing a shared or externally owned instance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The synchronous methods (`Read`, `Write`, `Exists`, `Delete`) now block on their async counterparts via `GetAwaiter().GetResult()`, which can introduce deadlocks or unnecessary thread blocking in some environments; consider either preserving a purely synchronous code path or documenting that these APIs are not safe for use on a context-bound thread (e.g., UI/main thread) and encouraging direct use of the async variants.
- Both `GodotFileStorage` and `FileStorage` always dispose the `IAsyncKeyLockManager`, even when it is provided via dependency injection; consider tracking whether the lock manager was created internally and only disposing it in that case, to avoid unintentionally disposing a shared or externally owned instance.
## Individual Comments
### Comment 1
<location path="GFramework.Godot/Storage/GodotFileStorage.cs" line_range="53-62" />
<code_context>
- if (File.Exists(path))
- File.Delete(path);
- }
+ DeleteAsync(key).GetAwaiter().GetResult();
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Using GetAwaiter().GetResult() on async methods can introduce deadlock risk in environments with a synchronization context.
Here the sync methods (Delete/Exists/Read/Write) are thin wrappers over their async versions via `GetAwaiter().GetResult()`. That pattern is especially risky when these APIs are used from frameworks like Godot or any context with a captured synchronization context.
To mitigate this, either:
1. Add `ConfigureAwait(false)` to all awaits in the async methods so they don’t attempt to resume on the original context, or
2. Make the sync methods truly synchronous (using sync I/O APIs) and treat the async methods as separate async implementations, rather than layering sync on top of async.
Even if you keep the current structure, adding `ConfigureAwait(false)` would significantly reduce the risk.
</issue_to_address>
### Comment 2
<location path="GFramework.Godot/Storage/GodotFileStorage.cs" line_range="66-69" />
<code_context>
+ ObjectDisposedException.ThrowIf(_disposed, this);
+ var path = ToPath(key);
+
+ await using (await _lockManager.AcquireLockAsync(path))
+ {
+ if (File.Exists(path))
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider consistently using ConfigureAwait(false) on awaited operations in this library-type storage code.
Since this storage layer may be consumed from UI/game threads or other contexts, these awaits (`AcquireLockAsync`, `File.ReadAllTextAsync`, `File.WriteAllTextAsync`, etc.) currently capture the caller’s synchronization context, which can lead to deadlocks or context-dependent behavior. In library code, it’s standard to use `ConfigureAwait(false)` on such awaits, e.g.:
```csharp
await using (await _lockManager.AcquireLockAsync(path).ConfigureAwait(false))
{
content = await File.ReadAllTextAsync(path, Encoding.UTF8).ConfigureAwait(false);
}
```
The same applies to the other async methods in `FileStorage`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- 添加了内部锁管理器所有权标识，防止外部传入的锁管理器被错误释放 - 在构造函数中正确初始化锁管理器的所有权状态 - 在Dispose方法中只释放内部创建的锁管理器，避免重复释放异常 - 为所有同步包装方法添加了ConfigureAwait(false)以避免死锁 - 更新了读取、写入、删除和检查存在的同步方法实现 - 为所有异步操作添加了适当的配置避免上下文切换问题 - 改进了Godot文件存储类的相同资源管理逻辑 - 为所有阻塞式同步方法添加了详细的XML注释警告说明
- 在 FileStorage 中添加 System.IO 和 System.Text 引用 - 在 GodotFileStorage 中整合所有必要的命名空间引用 - 统一并发和序列化接口的依赖注入方式 - 添加 Godot 特定的 FileAccess 类型别名 - 优化 Godot 扩展功能的引用结构
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
重构基于文件的存储实现，以使用异步键锁管理器、异步 I/O 和销毁保护机制，并为游戏文件存储添加原子写入支持。
新功能：
增强：
IDisposable支持及销毁检查，以便干净地释放锁管理器资源并防止销毁后的继续使用。Original summary in English
Summary by Sourcery
Refactor file-based storage implementations to use an async key-lock manager, async I/O, and disposal safeguards while adding atomic write support for game file storage.
New Features:
Enhancements: