NEO-54: fix in-memory inventory races with atomic TryMutateSnapshot
Re-read snapshot under per-player lock; route add/remove through TryMutateSnapshot so read-modify-write is serialized for in-memory and wrapped in one transaction for Postgres.pull/89/head
parent
ac3900c194
commit
dafe3cb92f
|
|
@ -8,4 +8,13 @@ public interface IPlayerInventoryStore
|
|||
|
||||
/// <summary>Atomically replaces the full snapshot for a known player; false when the player is not in the store.</summary>
|
||||
bool TryReplaceSnapshot(string playerId, PlayerInventorySnapshot snapshot);
|
||||
|
||||
/// <summary>
|
||||
/// Atomically reads the current snapshot and optionally persists a replacement under per-player lock (in-memory)
|
||||
/// or within one database transaction (Postgres).
|
||||
/// </summary>
|
||||
bool TryMutateSnapshot(
|
||||
string playerId,
|
||||
Func<PlayerInventorySnapshot, PlayerInventoryMutationWrite> mutator,
|
||||
out PlayerInventorySnapshot result);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ public sealed class InMemoryPlayerInventoryStore(IOptions<GamePositionOptions> o
|
|||
public bool TryGetSnapshot(string playerId, out PlayerInventorySnapshot snapshot)
|
||||
{
|
||||
var key = NormalizePlayerId(playerId);
|
||||
if (key.Length == 0 || !byPlayer.TryGetValue(key, out var existing))
|
||||
if (key.Length == 0)
|
||||
{
|
||||
snapshot = PlayerInventorySnapshot.Empty();
|
||||
return false;
|
||||
|
|
@ -31,7 +31,13 @@ public sealed class InMemoryPlayerInventoryStore(IOptions<GamePositionOptions> o
|
|||
|
||||
lock (playerLocks.GetOrAdd(key, _ => new object()))
|
||||
{
|
||||
snapshot = Clone(existing);
|
||||
if (!byPlayer.TryGetValue(key, out var current))
|
||||
{
|
||||
snapshot = PlayerInventorySnapshot.Empty();
|
||||
return false;
|
||||
}
|
||||
|
||||
snapshot = Clone(current);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
|
@ -40,18 +46,55 @@ public sealed class InMemoryPlayerInventoryStore(IOptions<GamePositionOptions> o
|
|||
public bool TryReplaceSnapshot(string playerId, PlayerInventorySnapshot snapshot)
|
||||
{
|
||||
var key = NormalizePlayerId(playerId);
|
||||
if (key.Length == 0 || !byPlayer.ContainsKey(key))
|
||||
if (key.Length == 0)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
lock (playerLocks.GetOrAdd(key, _ => new object()))
|
||||
{
|
||||
if (!byPlayer.ContainsKey(key))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
byPlayer[key] = Clone(snapshot);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public bool TryMutateSnapshot(
|
||||
string playerId,
|
||||
Func<PlayerInventorySnapshot, PlayerInventoryMutationWrite> mutator,
|
||||
out PlayerInventorySnapshot result)
|
||||
{
|
||||
var key = NormalizePlayerId(playerId);
|
||||
if (key.Length == 0)
|
||||
{
|
||||
result = PlayerInventorySnapshot.Empty();
|
||||
return false;
|
||||
}
|
||||
|
||||
lock (playerLocks.GetOrAdd(key, _ => new object()))
|
||||
{
|
||||
if (!byPlayer.TryGetValue(key, out var current))
|
||||
{
|
||||
result = PlayerInventorySnapshot.Empty();
|
||||
return false;
|
||||
}
|
||||
|
||||
var write = mutator(Clone(current));
|
||||
result = Clone(write.Value);
|
||||
if (write.Write)
|
||||
{
|
||||
byPlayer[key] = Clone(write.Value);
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
private static PlayerInventorySnapshot Clone(PlayerInventorySnapshot source) =>
|
||||
new()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -0,0 +1,5 @@
|
|||
namespace NeonSprawl.Server.Game.Items;
|
||||
|
||||
/// <summary>Result of an atomic inventory transform (NEO-54).</summary>
|
||||
/// <param name="Write"><c>true</c> to persist <see cref="Value"/>; <c>false</c> to leave storage unchanged.</param>
|
||||
public readonly record struct PlayerInventoryMutationWrite(bool Write, PlayerInventorySnapshot Value);
|
||||
|
|
@ -25,32 +25,36 @@ public static class PlayerInventoryOperations
|
|||
return Deny(playerId, store, PlayerInventoryReasonCodes.InvalidItem);
|
||||
}
|
||||
|
||||
if (!store.TryGetSnapshot(playerId, out var before))
|
||||
string? denyReason = null;
|
||||
if (!store.TryMutateSnapshot(
|
||||
playerId,
|
||||
before =>
|
||||
{
|
||||
var container = PlayerInventorySnapshot.ContainerFromSlotKind(def.InventorySlotKind);
|
||||
var slots = CloneSlots(before.GetSlots(container));
|
||||
var remaining = quantity;
|
||||
remaining = MergeIntoExistingStacks(slots, def.Id, remaining, def.StackMax);
|
||||
remaining = FillEmptySlots(slots, def.Id, remaining, def.StackMax);
|
||||
|
||||
if (remaining > 0)
|
||||
{
|
||||
denyReason = PlayerInventoryReasonCodes.InventoryFull;
|
||||
return new PlayerInventoryMutationWrite(Write: false, before);
|
||||
}
|
||||
|
||||
return new PlayerInventoryMutationWrite(Write: true, before.WithSlots(container, slots));
|
||||
},
|
||||
out var result))
|
||||
{
|
||||
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.StoreMissing, null, null);
|
||||
}
|
||||
|
||||
var container = PlayerInventorySnapshot.ContainerFromSlotKind(def.InventorySlotKind);
|
||||
var slots = CloneSlots(before.GetSlots(container));
|
||||
var remaining = quantity;
|
||||
remaining = MergeIntoExistingStacks(slots, def.Id, remaining, def.StackMax);
|
||||
remaining = FillEmptySlots(slots, def.Id, remaining, def.StackMax);
|
||||
|
||||
if (remaining > 0)
|
||||
if (denyReason is not null)
|
||||
{
|
||||
return new PlayerInventoryMutationOutcome(
|
||||
PlayerInventoryMutationKind.Denied,
|
||||
PlayerInventoryReasonCodes.InventoryFull,
|
||||
before);
|
||||
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.Denied, denyReason, result);
|
||||
}
|
||||
|
||||
var after = before.WithSlots(container, slots);
|
||||
if (!store.TryReplaceSnapshot(playerId, after))
|
||||
{
|
||||
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.StoreMissing, null, null);
|
||||
}
|
||||
|
||||
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.Applied, null, after);
|
||||
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.Applied, null, result);
|
||||
}
|
||||
|
||||
/// <summary>Removes <paramref name="quantity"/> from stacks of <paramref name="itemId"/> (lowest slot index first).</summary>
|
||||
|
|
@ -72,46 +76,50 @@ public static class PlayerInventoryOperations
|
|||
return Deny(playerId, store, PlayerInventoryReasonCodes.InvalidItem);
|
||||
}
|
||||
|
||||
if (!store.TryGetSnapshot(playerId, out var before))
|
||||
string? denyReason = null;
|
||||
if (!store.TryMutateSnapshot(
|
||||
playerId,
|
||||
before =>
|
||||
{
|
||||
var container = PlayerInventorySnapshot.ContainerFromSlotKind(def.InventorySlotKind);
|
||||
var slots = CloneSlots(before.GetSlots(container));
|
||||
var available = CountQuantity(slots, def.Id);
|
||||
if (available < quantity)
|
||||
{
|
||||
denyReason = PlayerInventoryReasonCodes.InsufficientQuantity;
|
||||
return new PlayerInventoryMutationWrite(Write: false, before);
|
||||
}
|
||||
|
||||
var remaining = quantity;
|
||||
for (var i = 0; i < slots.Length && remaining > 0; i++)
|
||||
{
|
||||
var slot = slots[i];
|
||||
if (slot.IsEmpty || !string.Equals(slot.ItemId, def.Id, StringComparison.Ordinal))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
var take = Math.Min(remaining, slot.Quantity);
|
||||
var left = slot.Quantity - take;
|
||||
slots[i] = left <= 0
|
||||
? new InventorySlotState(slot.SlotIndex, null, 0)
|
||||
: new InventorySlotState(slot.SlotIndex, def.Id, left);
|
||||
remaining -= take;
|
||||
}
|
||||
|
||||
return new PlayerInventoryMutationWrite(Write: true, before.WithSlots(container, slots));
|
||||
},
|
||||
out var result))
|
||||
{
|
||||
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.StoreMissing, null, null);
|
||||
}
|
||||
|
||||
var container = PlayerInventorySnapshot.ContainerFromSlotKind(def.InventorySlotKind);
|
||||
var slots = CloneSlots(before.GetSlots(container));
|
||||
var available = CountQuantity(slots, def.Id);
|
||||
if (available < quantity)
|
||||
if (denyReason is not null)
|
||||
{
|
||||
return new PlayerInventoryMutationOutcome(
|
||||
PlayerInventoryMutationKind.Denied,
|
||||
PlayerInventoryReasonCodes.InsufficientQuantity,
|
||||
before);
|
||||
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.Denied, denyReason, result);
|
||||
}
|
||||
|
||||
var remaining = quantity;
|
||||
for (var i = 0; i < slots.Length && remaining > 0; i++)
|
||||
{
|
||||
var slot = slots[i];
|
||||
if (slot.IsEmpty || !string.Equals(slot.ItemId, def.Id, StringComparison.Ordinal))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
var take = Math.Min(remaining, slot.Quantity);
|
||||
var left = slot.Quantity - take;
|
||||
slots[i] = left <= 0
|
||||
? new InventorySlotState(slot.SlotIndex, null, 0)
|
||||
: new InventorySlotState(slot.SlotIndex, def.Id, left);
|
||||
remaining -= take;
|
||||
}
|
||||
|
||||
var after = before.WithSlots(container, slots);
|
||||
if (!store.TryReplaceSnapshot(playerId, after))
|
||||
{
|
||||
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.StoreMissing, null, null);
|
||||
}
|
||||
|
||||
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.Applied, null, after);
|
||||
return new PlayerInventoryMutationOutcome(PlayerInventoryMutationKind.Applied, null, result);
|
||||
}
|
||||
|
||||
private static PlayerInventoryMutationOutcome Deny(string playerId, IPlayerInventoryStore store, string reasonCode)
|
||||
|
|
|
|||
|
|
@ -43,19 +43,63 @@ public sealed class PostgresPlayerInventoryStore(Npgsql.NpgsqlDataSource dataSou
|
|||
return false;
|
||||
}
|
||||
|
||||
ReplaceSnapshotRows(conn, norm, snapshot, tx);
|
||||
tx.Commit();
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public bool TryMutateSnapshot(
|
||||
string playerId,
|
||||
Func<PlayerInventorySnapshot, PlayerInventoryMutationWrite> mutator,
|
||||
out PlayerInventorySnapshot result)
|
||||
{
|
||||
var norm = NormalizePlayerId(playerId);
|
||||
if (norm.Length == 0)
|
||||
{
|
||||
result = PlayerInventorySnapshot.Empty();
|
||||
return false;
|
||||
}
|
||||
|
||||
PostgresPlayerInventoryBootstrap.EnsureSchema(dataSource);
|
||||
using var conn = dataSource.OpenConnection();
|
||||
using var tx = conn.BeginTransaction();
|
||||
if (!PlayerExists(conn, norm, tx))
|
||||
{
|
||||
tx.Rollback();
|
||||
result = PlayerInventorySnapshot.Empty();
|
||||
return false;
|
||||
}
|
||||
|
||||
var current = ReadSnapshot(conn, norm, tx);
|
||||
var write = mutator(current);
|
||||
result = write.Value;
|
||||
if (write.Write)
|
||||
{
|
||||
ReplaceSnapshotRows(conn, norm, write.Value, tx);
|
||||
}
|
||||
|
||||
tx.Commit();
|
||||
return true;
|
||||
}
|
||||
|
||||
private static void ReplaceSnapshotRows(
|
||||
Npgsql.NpgsqlConnection conn,
|
||||
string playerIdNormalized,
|
||||
PlayerInventorySnapshot snapshot,
|
||||
Npgsql.NpgsqlTransaction tx)
|
||||
{
|
||||
using (var del = new Npgsql.NpgsqlCommand(
|
||||
"DELETE FROM player_inventory WHERE player_id = @pid;",
|
||||
conn,
|
||||
tx))
|
||||
{
|
||||
del.Parameters.AddWithValue("pid", norm);
|
||||
del.Parameters.AddWithValue("pid", playerIdNormalized);
|
||||
del.ExecuteNonQuery();
|
||||
}
|
||||
|
||||
WriteOccupiedSlots(conn, norm, snapshot.BagSlots, InventoryContainerKind.Bag, tx);
|
||||
WriteOccupiedSlots(conn, norm, snapshot.EquipmentSlots, InventoryContainerKind.Equipment, tx);
|
||||
tx.Commit();
|
||||
return true;
|
||||
WriteOccupiedSlots(conn, playerIdNormalized, snapshot.BagSlots, InventoryContainerKind.Bag, tx);
|
||||
WriteOccupiedSlots(conn, playerIdNormalized, snapshot.EquipmentSlots, InventoryContainerKind.Equipment, tx);
|
||||
}
|
||||
|
||||
private static void WriteOccupiedSlots(
|
||||
|
|
@ -89,7 +133,10 @@ public sealed class PostgresPlayerInventoryStore(Npgsql.NpgsqlDataSource dataSou
|
|||
}
|
||||
}
|
||||
|
||||
private static PlayerInventorySnapshot ReadSnapshot(Npgsql.NpgsqlConnection conn, string playerIdNormalized)
|
||||
private static PlayerInventorySnapshot ReadSnapshot(
|
||||
Npgsql.NpgsqlConnection conn,
|
||||
string playerIdNormalized,
|
||||
Npgsql.NpgsqlTransaction? tx = null)
|
||||
{
|
||||
var bag = PlayerInventorySnapshot.CreateEmptySlots(PlayerInventorySnapshot.BagSlotCount);
|
||||
var equipment = PlayerInventorySnapshot.CreateEmptySlots(PlayerInventorySnapshot.EquipmentSlotCount);
|
||||
|
|
@ -101,7 +148,8 @@ public sealed class PostgresPlayerInventoryStore(Npgsql.NpgsqlDataSource dataSou
|
|||
WHERE player_id = @pid
|
||||
ORDER BY container_kind, slot_index;
|
||||
""",
|
||||
conn);
|
||||
conn,
|
||||
tx);
|
||||
cmd.Parameters.AddWithValue("pid", playerIdNormalized);
|
||||
using var reader = cmd.ExecuteReader();
|
||||
while (reader.Read())
|
||||
|
|
|
|||
Loading…
Reference in New Issue