Fix race condition for ModifyWalletObjectData

This commit is contained in:
nicolas.dorier
2025-07-14 14:56:42 +09:00
parent b445f32b65
commit 385b66653b
5 changed files with 63 additions and 15 deletions

View File

@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
@@ -42,6 +43,10 @@ namespace BTCPayServer.Data
public List<WalletObjectLinkData> Bs { get; set; }
public List<WalletObjectLinkData> As { get; set; }
#nullable enable
[Timestamp]
// With this, update of WalletObjectData will fail if the row was modified by another process
public uint XMin { get; set; }
public IEnumerable<(string type, string id, JObject? linkdata, JObject? objectdata)> GetLinks()
{
static JObject? AsJObj(string? data) => data is null ? null : JObject.Parse(data);

View File

@@ -966,6 +966,12 @@ namespace BTCPayServer.Migrations
b.Property<string>("Data")
.HasColumnType("JSONB");
b.Property<uint>("XMin")
.IsConcurrencyToken()
.ValueGeneratedOnAddOrUpdate()
.HasColumnType("xid")
.HasColumnName("xmin");
b.HasKey("WalletId", "Type", "Id");
b.HasIndex("Type", "Id");

View File

@@ -5,6 +5,7 @@ using System.Linq;
using System.Threading.Tasks;
using BTCPayServer.Abstractions.Models;
using BTCPayServer.Data;
using BTCPayServer.Services;
using BTCPayServer.Services.Invoices;
using BTCPayServer.Tests.Logging;
using Dapper;
@@ -49,6 +50,7 @@ namespace BTCPayServer.Tests
logs.Configure(_loggerFactory);
return new InvoiceRepository(CreateContextFactory(), new EventAggregator(logs));
}
public WalletRepository GetWalletRepository() => new (CreateContextFactory());
public ApplicationDbContext CreateContext() => CreateContextFactory().CreateContext();

View File

@@ -1,6 +1,7 @@
using System.Linq;
using System.Threading.Tasks;
using BTCPayServer.Payments;
using BTCPayServer.Services;
using Dapper;
using Microsoft.EntityFrameworkCore;
using NBitcoin;
@@ -20,6 +21,23 @@ namespace BTCPayServer.Tests
{
}
[Fact]
public async Task CanConcurrentlyModifyWalletObject()
{
var tester = CreateDBTester();
await tester.MigrateUntil();
var walletRepo = tester.GetWalletRepository();
var wid = new WalletObjectId(new WalletId("AAA", "ddd"), "a", "b");
var all = Enumerable.Range(0, 10)
.Select(i => walletRepo.ModifyWalletObjectData(wid, (o) => { o["idx"] = i; }))
.ToArray();
foreach (var task in all)
{
await task;
}
}
[Fact]
public async Task CanQueryMonitoredInvoices()
{
@@ -46,7 +64,7 @@ namespace BTCPayServer.Tests
}
await conn.ExecuteAsync("""
INSERT INTO "Invoices" ("Id", "Created", "Status","Currency") VALUES
INSERT INTO "Invoices" ("Id", "Created", "Status","Currency") VALUES
('BTCOnly', NOW(), 'New', 'USD'),
('LTCOnly', NOW(), 'New', 'USD'),
('LTCAndBTC', NOW(), 'New', 'USD'),

View File

@@ -408,7 +408,7 @@ namespace BTCPayServer.Services
public async Task EnsureWalletObjectLink(WalletObjectId a, WalletObjectId b, JObject? data = null)
{
await EnsureWalletObjectLink(NewWalletObjectLinkData(a, b, data));
}
}
public async Task EnsureWalletObjectLink(WalletObjectLinkData l)
{
await using var ctx = _ContextFactory.CreateContext();
@@ -522,19 +522,36 @@ namespace BTCPayServer.Services
{
ArgumentNullException.ThrowIfNull(id);
ArgumentNullException.ThrowIfNull(modify);
using var ctx = _ContextFactory.CreateContext();
var obj = await ctx.WalletObjects.FindAsync(id.WalletId.ToString(), id.Type, id.Id);
if (obj is null)
retry:
using (var ctx = _ContextFactory.CreateContext())
{
obj = NewWalletObjectData(id);
ctx.WalletObjects.Add(obj);
var obj = await ctx.WalletObjects.FindAsync(id.WalletId.ToString(), id.Type, id.Id);
if (obj is null)
{
obj = NewWalletObjectData(id);
ctx.WalletObjects.Add(obj);
}
var currentData = obj.Data is null ? new JObject() : JObject.Parse(obj.Data);
modify(currentData);
obj.Data = currentData.ToString();
if (obj.Data == "{}")
obj.Data = null;
try
{
await ctx.SaveChangesAsync();
}
// Race condition, retry
catch (DbUpdateConcurrencyException)
{
goto retry;
}
// Got created simultaneously
catch (DbUpdateException)
{
goto retry;
}
}
var currentData = obj.Data is null ? new JObject() : JObject.Parse(obj.Data);
modify(currentData);
obj.Data = currentData.ToString();
if (obj.Data == "{}")
obj.Data = null;
await ctx.SaveChangesAsync();
}
const int MaxLabelSize = 50;
@@ -566,7 +583,7 @@ namespace BTCPayServer.Services
public async Task AddWalletTransactionAttachments((WalletId walletId, string txId,
IEnumerable<Attachment> attachments, string type)[] reqs)
{
List<WalletObjectData> objs = new();
List<WalletObjectLinkData> links = new();
foreach ((WalletId walletId, string txId, IEnumerable<Attachment> attachments, string type) req in reqs)
@@ -627,7 +644,7 @@ namespace BTCPayServer.Services
await RemoveWalletObjectLink(labelObjId, id);
}
}
public async Task<bool> RemoveWalletLabels(WalletId id, params string[] labels)
{
ArgumentNullException.ThrowIfNull(id);