NEO-146: fix in-memory create/complete lock ordering

Nest player and instance locks in TryCreateActive, TryMarkComplete, and
TryClearInstance so concurrent create and complete cannot leave a stale
active index or mismatched snapshot.
pull/187/head
VinPropane 2026-06-24 21:05:56 -04:00
parent 7740d2145f
commit 6c4221072a
3 changed files with 102 additions and 55 deletions

View File

@ -68,7 +68,7 @@ Durable per-player **`ContractInstance`** rows (active/completed) and append-onl
- **Types:** `ContractInstanceStatus`, `ContractInstanceState`, `ContractInstanceIds`, `ContractInstanceReasonCodes`, `ContractOutcomeRow`, `ContractOutcomeIds`.
- **Stores:** `IContractInstanceStore` / `IContractOutcomeStore`; in-memory + Postgres (`V011`, `V012`) when connection string set; partial unique index + app-level one-active enforcement.
- **DI:** `AddContractInstanceStores` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`.
- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`871` tests green).
- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`872` tests green).
- **In-memory outcome guard:** `InMemoryContractOutcomeStore.TryAppend` denies when **`IContractInstanceStore.TryGet`** is false (Postgres FK + player/instance SQL guard parity).
- **Docs:** `server/README.md` contract instance + outcome store sections.
- **Bruno:** `bruno/neon-sprawl-server/contract-stores/` health smoke (startup-only; no contract HTTP in E7M4-03).

View File

@ -227,6 +227,44 @@ public sealed class InMemoryContractInstanceStoreTests
Assert.False(created);
}
[Fact]
public async Task TryCreateActiveAndTryMarkComplete_ShouldKeepActiveIndexConsistent_WhenRacingOnSameInstanceId()
{
// Arrange
var store = CreateStore();
// Act
await Task.WhenAll(
Enumerable.Range(0, 32).Select(i => Task.Run(() =>
{
if (i % 2 == 0)
{
_ = store.TryCreateActive(
PlayerId,
InstanceId,
TemplateId,
SeedBucket,
IssuedAt,
out _);
return;
}
_ = store.TryMarkComplete(PlayerId, InstanceId, CompletedAt, out _);
})));
// Assert
if (store.TryGetActiveForPlayer(PlayerId, out var active))
{
Assert.True(store.TryGet(PlayerId, active.ContractInstanceId, out var row));
Assert.Equal(ContractInstanceStatus.Active, row.Status);
}
if (store.TryGet(PlayerId, InstanceId, out var final) &&
final.Status == ContractInstanceStatus.Completed)
{
Assert.False(store.TryGetActiveForPlayer(PlayerId, out _));
}
}
[Fact]
public async Task Host_ShouldResolveContractInstanceStoresFromDi()
{

View File

@ -153,36 +153,39 @@ public sealed class InMemoryContractInstanceStore(IOptions<GamePositionOptions>
lock (instanceLocks.GetOrAdd(PlayerLockKey(player), _ => new object()))
{
if (activeInstanceByPlayer.TryGetValue(player, out var existingActiveId))
lock (instanceLocks.GetOrAdd(ContractInstanceIds.MakeInstanceKey(player, instanceId), _ => new object()))
{
if (byInstanceId.TryGetValue(existingActiveId, out var existingActive) &&
existingActive.Status == ContractInstanceStatus.Active)
if (activeInstanceByPlayer.TryGetValue(player, out var existingActiveId))
{
snapshot = existingActive.ToSnapshot();
if (byInstanceId.TryGetValue(existingActiveId, out var existingActive) &&
existingActive.Status == ContractInstanceStatus.Active)
{
snapshot = existingActive.ToSnapshot();
return false;
}
activeInstanceByPlayer.TryRemove(player, out _);
}
if (byInstanceId.TryGetValue(instanceId, out var existing))
{
snapshot = existing.ToSnapshot();
return false;
}
activeInstanceByPlayer.TryRemove(player, out _);
var row = new InstanceRow(
instanceId,
normalizedTemplateId,
player,
ContractInstanceStatus.Active,
normalizedSeedBucket,
issuedAt,
null);
byInstanceId[instanceId] = row;
activeInstanceByPlayer[player] = instanceId;
snapshot = row.ToSnapshot();
return true;
}
if (byInstanceId.TryGetValue(instanceId, out var existing))
{
snapshot = existing.ToSnapshot();
return false;
}
var row = new InstanceRow(
instanceId,
normalizedTemplateId,
player,
ContractInstanceStatus.Active,
normalizedSeedBucket,
issuedAt,
null);
byInstanceId[instanceId] = row;
activeInstanceByPlayer[player] = instanceId;
snapshot = row.ToSnapshot();
return true;
}
}
@ -207,30 +210,33 @@ public sealed class InMemoryContractInstanceStore(IOptions<GamePositionOptions>
return false;
}
lock (instanceLocks.GetOrAdd(lockKey, _ => new object()))
lock (instanceLocks.GetOrAdd(PlayerLockKey(player), _ => new object()))
{
if (!byInstanceId.TryGetValue(instanceId, out var row) ||
!string.Equals(row.PlayerId, player, StringComparison.Ordinal))
lock (instanceLocks.GetOrAdd(lockKey, _ => new object()))
{
return false;
}
if (!byInstanceId.TryGetValue(instanceId, out var row) ||
!string.Equals(row.PlayerId, player, StringComparison.Ordinal))
{
return false;
}
if (row.Status == ContractInstanceStatus.Completed)
{
if (row.Status == ContractInstanceStatus.Completed)
{
snapshot = row.ToSnapshot();
return false;
}
if (row.Status != ContractInstanceStatus.Active)
{
snapshot = row.ToSnapshot();
return false;
}
row.MarkCompleted(completedAt);
activeInstanceByPlayer.TryRemove(player, out _);
snapshot = row.ToSnapshot();
return false;
return true;
}
if (row.Status != ContractInstanceStatus.Active)
{
snapshot = row.ToSnapshot();
return false;
}
row.MarkCompleted(completedAt);
activeInstanceByPlayer.TryRemove(player, out _);
snapshot = row.ToSnapshot();
return true;
}
}
@ -250,21 +256,24 @@ public sealed class InMemoryContractInstanceStore(IOptions<GamePositionOptions>
return false;
}
lock (instanceLocks.GetOrAdd(lockKey, _ => new object()))
lock (instanceLocks.GetOrAdd(PlayerLockKey(player), _ => new object()))
{
if (!byInstanceId.TryGetValue(instanceId, out var row) ||
!string.Equals(row.PlayerId, player, StringComparison.Ordinal))
lock (instanceLocks.GetOrAdd(lockKey, _ => new object()))
{
return false;
}
if (!byInstanceId.TryGetValue(instanceId, out var row) ||
!string.Equals(row.PlayerId, player, StringComparison.Ordinal))
{
return false;
}
_ = byInstanceId.TryRemove(instanceId, out _);
if (row.Status == ContractInstanceStatus.Active)
{
activeInstanceByPlayer.TryRemove(player, out _);
}
_ = byInstanceId.TryRemove(instanceId, out _);
if (row.Status == ContractInstanceStatus.Active)
{
activeInstanceByPlayer.TryRemove(player, out _);
}
return true;
return true;
}
}
}
}