NEO-146: fix stale active index race in TryGetActiveForPlayer
Read the active index under the player lock and remove stale entries only when the indexed instance id still matches, preventing concurrent get/complete/reissue from dropping a newer active pointer.pull/187/head
parent
a64f7100a2
commit
1cf80a4e4e
|
|
@ -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 (`874` tests green).
|
||||
- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`875` 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).
|
||||
|
|
|
|||
|
|
@ -265,6 +265,57 @@ public sealed class InMemoryContractInstanceStoreTests
|
|||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task TryGetActiveForPlayer_ShouldKeepActiveIndexConsistent_WhenRacingWithCompleteAndReissue()
|
||||
{
|
||||
// Arrange
|
||||
var store = CreateStore();
|
||||
Assert.True(store.TryCreateActive(
|
||||
PlayerId,
|
||||
InstanceId,
|
||||
TemplateId,
|
||||
SeedBucket,
|
||||
IssuedAt,
|
||||
out _));
|
||||
// Act
|
||||
await Task.WhenAll(
|
||||
Enumerable.Range(0, 32).Select(i => Task.Run(() =>
|
||||
{
|
||||
if (i % 3 == 0)
|
||||
{
|
||||
_ = store.TryGetActiveForPlayer(PlayerId, out _);
|
||||
return;
|
||||
}
|
||||
|
||||
if (i % 3 == 1)
|
||||
{
|
||||
_ = store.TryMarkComplete(PlayerId, InstanceId, CompletedAt, out _);
|
||||
return;
|
||||
}
|
||||
|
||||
_ = store.TryCreateActive(
|
||||
PlayerId,
|
||||
SecondInstanceId,
|
||||
TemplateId,
|
||||
SeedBucket,
|
||||
IssuedAt.AddDays(1),
|
||||
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);
|
||||
}
|
||||
|
||||
Assert.False(
|
||||
store.TryGet(PlayerId, InstanceId, out var first) &&
|
||||
store.TryGet(PlayerId, SecondInstanceId, out var second) &&
|
||||
first.Status == ContractInstanceStatus.Active &&
|
||||
second.Status == ContractInstanceStatus.Active);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TryClearInstance_ShouldReturnTrue_WhenRowMissing()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -108,18 +108,18 @@ public sealed class InMemoryContractInstanceStore(IOptions<GamePositionOptions>
|
|||
return false;
|
||||
}
|
||||
|
||||
if (!activeInstanceByPlayer.TryGetValue(player, out var instanceId))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
lock (instanceLocks.GetOrAdd(PlayerLockKey(player), _ => new object()))
|
||||
{
|
||||
if (!activeInstanceByPlayer.TryGetValue(player, out var instanceId))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!byInstanceId.TryGetValue(instanceId, out var row) ||
|
||||
row.Status != ContractInstanceStatus.Active ||
|
||||
!string.Equals(row.PlayerId, player, StringComparison.Ordinal))
|
||||
{
|
||||
activeInstanceByPlayer.TryRemove(player, out _);
|
||||
activeInstanceByPlayer.TryRemove(new KeyValuePair<string, string>(player, instanceId));
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
@ -164,7 +164,7 @@ public sealed class InMemoryContractInstanceStore(IOptions<GamePositionOptions>
|
|||
return false;
|
||||
}
|
||||
|
||||
activeInstanceByPlayer.TryRemove(player, out _);
|
||||
activeInstanceByPlayer.TryRemove(new KeyValuePair<string, string>(player, existingActiveId));
|
||||
}
|
||||
|
||||
if (byInstanceId.TryGetValue(instanceId, out var existing))
|
||||
|
|
@ -233,7 +233,7 @@ public sealed class InMemoryContractInstanceStore(IOptions<GamePositionOptions>
|
|||
}
|
||||
|
||||
row.MarkCompleted(completedAt);
|
||||
activeInstanceByPlayer.TryRemove(player, out _);
|
||||
activeInstanceByPlayer.TryRemove(new KeyValuePair<string, string>(player, instanceId));
|
||||
snapshot = row.ToSnapshot();
|
||||
return true;
|
||||
}
|
||||
|
|
@ -269,7 +269,7 @@ public sealed class InMemoryContractInstanceStore(IOptions<GamePositionOptions>
|
|||
_ = byInstanceId.TryRemove(instanceId, out _);
|
||||
if (row.Status == ContractInstanceStatus.Active)
|
||||
{
|
||||
activeInstanceByPlayer.TryRemove(player, out _);
|
||||
activeInstanceByPlayer.TryRemove(new KeyValuePair<string, string>(player, instanceId));
|
||||
}
|
||||
|
||||
return true;
|
||||
|
|
|
|||
Loading…
Reference in New Issue