mirror of
https://github.com/GeWuYou/GFramework.git
synced 2026-05-07 00:39:00 +08:00
fix(game): 修复设置迁移缓存并发一致性
- 修复 SettingsModel 迁移注册与缓存重建的并发竞争 - 新增 SettingsModel 并发回归测试并更新 ai-plan 跟踪
This commit is contained in:
parent
a0cc418e05
commit
5353d5bd45
@ -1,3 +1,5 @@
|
|||||||
|
using System.Reflection;
|
||||||
|
using System.Threading;
|
||||||
using GFramework.Core.Abstractions.Architectures;
|
using GFramework.Core.Abstractions.Architectures;
|
||||||
using GFramework.Core.Abstractions.Lifecycle;
|
using GFramework.Core.Abstractions.Lifecycle;
|
||||||
using GFramework.Core.Abstractions.Rule;
|
using GFramework.Core.Abstractions.Rule;
|
||||||
@ -109,6 +111,66 @@ public sealed class SettingsModelTests
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Test]
|
||||||
|
public async Task RegisterMigration_During_Cache_Rebuild_Should_Not_Leave_Stale_Type_Cache()
|
||||||
|
{
|
||||||
|
var locationProvider = new TestDataLocationProvider();
|
||||||
|
var repository = new FakeSettingsDataRepository();
|
||||||
|
var model = new SettingsModel<FakeSettingsDataRepository>(locationProvider, repository);
|
||||||
|
((IContextAware)model).SetContext(new Mock<IArchitectureContext>(MockBehavior.Loose).Object);
|
||||||
|
|
||||||
|
_ = model.GetData<TestLatestSettingsData>();
|
||||||
|
((IInitializable)model).Initialize();
|
||||||
|
|
||||||
|
model.RegisterMigration(new TestLatestSettingsMigrationV1ToV2());
|
||||||
|
|
||||||
|
repository.Stored["TestLatestSettingsData"] = new TestLatestSettingsData
|
||||||
|
{
|
||||||
|
Version = 1,
|
||||||
|
Value = "legacy"
|
||||||
|
};
|
||||||
|
|
||||||
|
var lockField = typeof(SettingsModel<FakeSettingsDataRepository>)
|
||||||
|
.GetField("_migrationMapLock", BindingFlags.Instance | BindingFlags.NonPublic);
|
||||||
|
Assert.That(lockField, Is.Not.Null);
|
||||||
|
|
||||||
|
var migrationMapLock = lockField!.GetValue(model);
|
||||||
|
Assert.That(migrationMapLock, Is.Not.Null);
|
||||||
|
|
||||||
|
Task initializeTask;
|
||||||
|
Task registerTask;
|
||||||
|
lock (migrationMapLock!)
|
||||||
|
{
|
||||||
|
initializeTask = Task.Run(() => model.InitializeAsync());
|
||||||
|
registerTask = Task.Run(() => model.RegisterMigration(new TestLatestSettingsMigrationV2ToV3()));
|
||||||
|
|
||||||
|
Thread.Sleep(50);
|
||||||
|
|
||||||
|
Assert.Multiple(() =>
|
||||||
|
{
|
||||||
|
Assert.That(initializeTask.IsCompleted, Is.False);
|
||||||
|
Assert.That(registerTask.IsCompleted, Is.False);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
await Task.WhenAll(initializeTask, registerTask);
|
||||||
|
|
||||||
|
repository.Stored["TestLatestSettingsData"] = new TestLatestSettingsData
|
||||||
|
{
|
||||||
|
Version = 1,
|
||||||
|
Value = "legacy"
|
||||||
|
};
|
||||||
|
|
||||||
|
await model.InitializeAsync();
|
||||||
|
|
||||||
|
var current = model.GetData<TestLatestSettingsData>();
|
||||||
|
Assert.Multiple(() =>
|
||||||
|
{
|
||||||
|
Assert.That(current.Version, Is.EqualTo(3));
|
||||||
|
Assert.That(current.Value, Is.EqualTo("legacy-migrated-v3"));
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
private sealed class TestSettingsData : ISettingsData
|
private sealed class TestSettingsData : ISettingsData
|
||||||
{
|
{
|
||||||
public string Value { get; set; } = "default";
|
public string Value { get; set; } = "default";
|
||||||
@ -199,6 +261,25 @@ public sealed class SettingsModelTests
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private sealed class TestLatestSettingsMigrationV2ToV3 : ISettingsMigration
|
||||||
|
{
|
||||||
|
public Type SettingsType => typeof(TestLatestSettingsData);
|
||||||
|
|
||||||
|
public int FromVersion => 2;
|
||||||
|
|
||||||
|
public int ToVersion => 3;
|
||||||
|
|
||||||
|
public ISettingsSection Migrate(ISettingsSection oldData)
|
||||||
|
{
|
||||||
|
var data = (TestLatestSettingsData)oldData;
|
||||||
|
return new TestLatestSettingsData
|
||||||
|
{
|
||||||
|
Version = 3,
|
||||||
|
Value = $"{data.Value}-v3"
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private sealed class FakeSettingsDataRepository : ISettingsDataRepository
|
private sealed class FakeSettingsDataRepository : ISettingsDataRepository
|
||||||
{
|
{
|
||||||
public Dictionary<string, Type> RegisteredTypes { get; } = new(StringComparer.Ordinal);
|
public Dictionary<string, Type> RegisteredTypes { get; } = new(StringComparer.Ordinal);
|
||||||
|
|||||||
@ -29,6 +29,7 @@ public class SettingsModel<TRepository>(IDataLocationProvider? locationProvider,
|
|||||||
|
|
||||||
private readonly ConcurrentDictionary<Type, ISettingsData> _data = new();
|
private readonly ConcurrentDictionary<Type, ISettingsData> _data = new();
|
||||||
private readonly ConcurrentDictionary<Type, Dictionary<int, ISettingsMigration>> _migrationCache = new();
|
private readonly ConcurrentDictionary<Type, Dictionary<int, ISettingsMigration>> _migrationCache = new();
|
||||||
|
private readonly object _migrationMapLock = new();
|
||||||
private readonly ConcurrentDictionary<(Type type, int from), ISettingsMigration> _migrations = new();
|
private readonly ConcurrentDictionary<(Type type, int from), ISettingsMigration> _migrations = new();
|
||||||
private volatile bool _initialized;
|
private volatile bool _initialized;
|
||||||
|
|
||||||
@ -124,6 +125,10 @@ public class SettingsModel<TRepository>(IDataLocationProvider? locationProvider,
|
|||||||
/// <exception cref="InvalidOperationException">
|
/// <exception cref="InvalidOperationException">
|
||||||
/// 同一设置类型与源版本已经注册过迁移器时抛出。
|
/// 同一设置类型与源版本已经注册过迁移器时抛出。
|
||||||
/// </exception>
|
/// </exception>
|
||||||
|
/// <remarks>
|
||||||
|
/// 迁移注册表与按类型缓存的版本映射需要保持一致;因此注册与 cache miss 时的缓存重建
|
||||||
|
/// 统一通过 <see cref="_migrationMapLock" /> 串行化,避免并发加载把旧快照重新写回缓存。
|
||||||
|
/// </remarks>
|
||||||
public ISettingsModel RegisterMigration(ISettingsMigration migration)
|
public ISettingsModel RegisterMigration(ISettingsMigration migration)
|
||||||
{
|
{
|
||||||
ArgumentNullException.ThrowIfNull(migration);
|
ArgumentNullException.ThrowIfNull(migration);
|
||||||
@ -135,13 +140,17 @@ public class SettingsModel<TRepository>(IDataLocationProvider? locationProvider,
|
|||||||
migration.ToVersion,
|
migration.ToVersion,
|
||||||
nameof(migration));
|
nameof(migration));
|
||||||
|
|
||||||
if (!_migrations.TryAdd((migration.SettingsType, migration.FromVersion), migration))
|
lock (_migrationMapLock)
|
||||||
{
|
{
|
||||||
throw new InvalidOperationException(
|
if (!_migrations.TryAdd((migration.SettingsType, migration.FromVersion), migration))
|
||||||
$"Duplicate settings migration registration for {migration.SettingsType.Name} from version {migration.FromVersion}.");
|
{
|
||||||
|
throw new InvalidOperationException(
|
||||||
|
$"Duplicate settings migration registration for {migration.SettingsType.Name} from version {migration.FromVersion}.");
|
||||||
|
}
|
||||||
|
|
||||||
|
_migrationCache.TryRemove(migration.SettingsType, out _);
|
||||||
}
|
}
|
||||||
|
|
||||||
_migrationCache.TryRemove(migration.SettingsType, out _);
|
|
||||||
return this;
|
return this;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -310,18 +319,28 @@ public class SettingsModel<TRepository>(IDataLocationProvider? locationProvider,
|
|||||||
/// <remarks>
|
/// <remarks>
|
||||||
/// 该方法按设置类型缓存迁移表,并始终以 <paramref name="latestData" /> 的版本作为目标运行时版本,
|
/// 该方法按设置类型缓存迁移表,并始终以 <paramref name="latestData" /> 的版本作为目标运行时版本,
|
||||||
/// 避免把旧文件中的版本号误当成当前版本。具体的缺链、版本一致性与前进性校验都委托给
|
/// 避免把旧文件中的版本号误当成当前版本。具体的缺链、版本一致性与前进性校验都委托给
|
||||||
/// <see cref="VersionedMigrationRunner" /> 统一处理。
|
/// <see cref="VersionedMigrationRunner" /> 统一处理。缓存重建与迁移注册共用
|
||||||
|
/// <see cref="_migrationMapLock" />,确保运行中的初始化不会把过期迁移快照写回缓存。
|
||||||
/// </remarks>
|
/// </remarks>
|
||||||
private ISettingsData MigrateIfNeeded(ISettingsData data, ISettingsData latestData)
|
private ISettingsData MigrateIfNeeded(ISettingsData data, ISettingsData latestData)
|
||||||
{
|
{
|
||||||
var type = data.GetType();
|
var type = data.GetType();
|
||||||
if (!_migrationCache.TryGetValue(type, out var versionMap))
|
Dictionary<int, ISettingsMigration> versionMap;
|
||||||
|
lock (_migrationMapLock)
|
||||||
{
|
{
|
||||||
versionMap = _migrations
|
if (!_migrationCache.TryGetValue(type, out var cachedVersionMap))
|
||||||
.Where(kv => kv.Key.type == type)
|
{
|
||||||
.ToDictionary(kv => kv.Key.from, kv => kv.Value);
|
// cache miss 与 RegisterMigration 共用同一把锁,避免注册新迁移后又被旧快照覆盖回缓存。
|
||||||
|
versionMap = _migrations
|
||||||
|
.Where(kv => kv.Key.type == type)
|
||||||
|
.ToDictionary(kv => kv.Key.from, kv => kv.Value);
|
||||||
|
|
||||||
_migrationCache[type] = versionMap;
|
_migrationCache[type] = versionMap;
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
versionMap = cachedVersionMap;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return VersionedMigrationRunner.MigrateToTargetVersion(
|
return VersionedMigrationRunner.MigrateToTargetVersion(
|
||||||
|
|||||||
@ -14,7 +14,7 @@
|
|||||||
`ai-plan/public/data-repository-persistence/`
|
`ai-plan/public/data-repository-persistence/`
|
||||||
- 第一轮 settings / persistence / serialization 修复、测试与文档同步已完成,并收入主题内 `archive/`
|
- 第一轮 settings / persistence / serialization 修复、测试与文档同步已完成,并收入主题内 `archive/`
|
||||||
- 已完成 `SettingsModel` / `SaveRepository<T>` 共享迁移执行器收敛与契约补强
|
- 已完成 `SettingsModel` / `SaveRepository<T>` 共享迁移执行器收敛与契约补强
|
||||||
- 已完成 PR #260 的 review follow-up:迁移链快照一致性、XML docs 补齐与文档安全示例修正
|
- 已完成 PR #260 的追加 review follow-up:`SettingsModel` 迁移缓存并发一致性
|
||||||
- 下一轮需要继续评估 codec / persistence pipeline 边界
|
- 下一轮需要继续评估 codec / persistence pipeline 边界
|
||||||
|
|
||||||
## 当前状态摘要
|
## 当前状态摘要
|
||||||
@ -35,6 +35,8 @@
|
|||||||
- `SaveRepository<T>` 继续在 `LoadAsync(slot)` 期间迁移并回写,但其核心链式校验已与设置迁移共用同一实现
|
- `SaveRepository<T>` 继续在 `LoadAsync(slot)` 期间迁移并回写,但其核心链式校验已与设置迁移共用同一实现
|
||||||
- PR #260 review follow-up 已完成:`VersionedMigrationRunner` / `SettingsModel` 的 XML 异常契约已补齐,
|
- PR #260 review follow-up 已完成:`VersionedMigrationRunner` / `SettingsModel` 的 XML 异常契约已补齐,
|
||||||
`SaveRepository<T>` 单次加载已切换为迁移表快照,避免并发注册期间读取变化中的迁移链
|
`SaveRepository<T>` 单次加载已切换为迁移表快照,避免并发注册期间读取变化中的迁移链
|
||||||
|
- `SettingsModel` 现已通过 `_migrationMapLock` 串行化迁移注册与 cache miss 时的按类型缓存重建,
|
||||||
|
避免并发注册把旧快照重新写回 `_migrationCache`
|
||||||
- `docs/zh-CN/game/index.md` 当前仍承担最低接入示例,因此其中的 `JsonSerializer` 配置必须避免鼓励对
|
- `docs/zh-CN/game/index.md` 当前仍承担最低接入示例,因此其中的 `JsonSerializer` 配置必须避免鼓励对
|
||||||
用户可篡改存档启用不受限的多态反序列化
|
用户可篡改存档启用不受限的多态反序列化
|
||||||
|
|
||||||
@ -61,6 +63,9 @@
|
|||||||
- 已完成 PR #260 follow-up,并新增定向回归测试锁定迁移快照与失败不污染持久化数据的约束
|
- 已完成 PR #260 follow-up,并新增定向回归测试锁定迁移快照与失败不污染持久化数据的约束
|
||||||
- `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests|FullyQualifiedName~PersistenceTests" -m:1 -nodeReuse:false`
|
- `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests|FullyQualifiedName~PersistenceTests" -m:1 -nodeReuse:false`
|
||||||
已通过(21/21)
|
已通过(21/21)
|
||||||
|
- 已新增 `SettingsModelTests` 并发回归测试,锁定迁移注册与 cache miss 重建不会留下 stale cache
|
||||||
|
- `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests" -m:1 -nodeReuse:false`
|
||||||
|
已通过(5/5)
|
||||||
- 本次定向验证过程中出现的 analyzer warning 来自仓库既有代码,不属于本轮新增问题
|
- 本次定向验证过程中出现的 analyzer warning 来自仓库既有代码,不属于本轮新增问题
|
||||||
|
|
||||||
## 下一步
|
## 下一步
|
||||||
|
|||||||
@ -107,3 +107,24 @@
|
|||||||
|
|
||||||
1. 回到 codec / persistence pipeline 边界评估
|
1. 回到 codec / persistence pipeline 边界评估
|
||||||
2. 继续判断压缩、加密、元数据与备份策略是否需要新的 dedicated pipeline abstraction
|
2. 继续判断压缩、加密、元数据与备份策略是否需要新的 dedicated pipeline abstraction
|
||||||
|
|
||||||
|
### 阶段:SettingsModel 迁移缓存并发修复(RP-001)
|
||||||
|
|
||||||
|
- 重新使用 `$gframework-pr-review` 复核 PR #260 后确认:此前遗漏了一条仍然 open 的 `SettingsModel.cs` major comment,
|
||||||
|
问题点不是迁移执行本身,而是 `_migrations` 与 `_migrationCache` 在并发注册和 cache miss 重建交错时,可能把旧快照写回缓存
|
||||||
|
- 确认该 comment 来自 `2026-04-20T04:23:09Z` 的 CodeRabbit review run;当前修复策略采用同一把私有锁串行化:
|
||||||
|
- `RegisterMigration` 中的 `_migrations.TryAdd(...)` 与 `_migrationCache.TryRemove(...)`
|
||||||
|
- `MigrateIfNeeded` 在 cache miss 时按类型重建 `versionMap` 并写回 `_migrationCache`
|
||||||
|
- 同步补充源码注释与 XML remarks,明确运行时注册与缓存重建共享同一并发语义
|
||||||
|
- 计划新增 `SettingsModelTests` 回归测试,验证 cache rebuild 与运行时注册在同一把锁前排队后,后续初始化能看到新增迁移而不会留下 stale cache
|
||||||
|
|
||||||
|
### 验证:SettingsModel 迁移缓存并发修复
|
||||||
|
|
||||||
|
1. `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~SettingsModelTests" -m:1 -nodeReuse:false` 已通过(5/5)
|
||||||
|
2. 本轮验证中未再出现 `SettingsModel` 新增的 nullable warning;输出中的 analyzer warning 仍为仓库既有项加上新的 `MA0158`
|
||||||
|
建议,后者来自本轮新增对象锁
|
||||||
|
|
||||||
|
### 下一步:SettingsModel 迁移缓存并发修复
|
||||||
|
|
||||||
|
1. 若继续收口 analyzer 反馈,可评估是否将 `_migrationMapLock` 升级为 `System.Threading.Lock`,同时保留可验证的并发回归测试策略
|
||||||
|
2. 否则恢复到 codec / persistence pipeline 边界评估
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user