fix(service): run inbound deletes and depleted-client purges on the serial traffic writer (#6912)

Inbound and traffic writes are serialised on one goroutine because the traffic
poll locks inbounds then client_traffics while an admin mutation does the
reverse, which Postgres aborts as a deadlock (40P01). Deleting an inbound and
purging depleted clients still opened their own transactions, so they could
deadlock the poll, and a delete committing between a concurrent edit's read
and its Save let GORM's Save fall back to CREATE and resurrect the inbound.
Both now go through runSerializedTx like every other inbound write.
This commit is contained in:
25f2002654
2026-10-24 05:06:03 +02:00
committed by GitHub
parent 5a135d2adb
commit 9f0f2b3802
3 changed files with 125 additions and 3 deletions
+1 -1
View File
@@ -1651,7 +1651,7 @@ func (s *InboundService) delInbound(id int) (bool, func(), error) {
logger.Debug("DelInbound: inbound not found, id:", id)
}
if err := db.Transaction(func(tx *gorm.DB) error {
if err := runSerializedTx(func(tx *gorm.DB) error {
if err := s.clientService.DetachInbound(tx, id); err != nil {
return err
}
+1 -2
View File
@@ -897,9 +897,8 @@ func (s *InboundService) ResetInboundTraffic(id int) error {
}
func (s *InboundService) DelDepletedClients(id int) (err error) {
db := database.GetDB()
var deletedInbounds []model.Inbound
err = db.Transaction(func(tx *gorm.DB) error {
err = runSerializedTx(func(tx *gorm.DB) error {
// Collect depleted emails globally — a shared-email row owned by one
// inbound depletes every sibling that lists the email.
now := time.Now().Unix() * 1000
+123
View File
@@ -4,6 +4,12 @@ import (
"sync/atomic"
"testing"
"time"
"gorm.io/gorm"
"github.com/mhsanaei/3x-ui/v3/internal/database"
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
"github.com/mhsanaei/3x-ui/v3/internal/xray"
)
func TestTrafficWriterStartStopStartAcceptsWrites(t *testing.T) {
@@ -188,3 +194,120 @@ func waitTrafficWriterErr(t *testing.T, ch <-chan error) error {
return nil
}
}
// parkSerializedWriter holds the single writer inside an open transaction, so a write handed to it stays queued.
func parkSerializedWriter(t *testing.T) (release func()) {
t.Helper()
entered := make(chan struct{})
releaseCh := make(chan struct{})
go func() {
_ = submitTrafficWrite(func() error {
return database.GetDB().Transaction(func(tx *gorm.DB) error {
close(entered)
<-releaseCh
return nil
})
})
}()
waitTrafficWriterSignal(t, entered, "serial writer did not enter its transaction")
return func() { close(releaseCh) }
}
// waitForQueuedWrite reports whether a write reached the writer queue instead of opening its own transaction.
func waitForQueuedWrite() bool {
deadline := time.Now().Add(time.Second)
for time.Now().Before(deadline) {
twMu.Lock()
queued := len(twQueue) > 0
twMu.Unlock()
if queued {
return true
}
time.Sleep(10 * time.Millisecond)
}
return false
}
func waitWriterResult(t *testing.T, done <-chan error) error {
t.Helper()
select {
case err := <-done:
return err
case <-time.After(5 * time.Second):
t.Fatal("timed out waiting for the queued write to finish")
return nil
}
}
// The poll and an admin mutation lock inbounds and client_traffics in opposite order, so a delete in its
// own transaction can deadlock the poll on Postgres (40P01) and resurrect an inbound an edit just saved.
func TestDelInboundRunsOnTheSerialWriter(t *testing.T) {
startSerializedWriter(t)
db := initTrafficTestDB(t)
svc := &InboundService{}
ib := mkInbound(t, 54101, model.VLESS, `{"clients": []}`)
release := parkSerializedWriter(t)
done := make(chan error, 1)
go func() {
_, _, err := svc.delInbound(ib.Id)
done <- err
}()
queued := waitForQueuedWrite()
release()
if err := waitWriterResult(t, done); err != nil {
t.Fatalf("delInbound: %v", err)
}
if !queued {
t.Fatal("the inbound delete bypassed the serial traffic writer and opened its own transaction")
}
var remaining int64
if err := db.Model(&model.Inbound{}).Where("id = ?", ib.Id).Count(&remaining).Error; err != nil {
t.Fatalf("count inbounds: %v", err)
}
if remaining != 0 {
t.Fatalf("inbound %d still present after the delete", ib.Id)
}
}
// The purge writes inbounds and client_traffics in the opposite order to the poll, so off the writer it
// can deadlock the poll on Postgres (40P01).
func TestDelDepletedClientsRunsOnTheSerialWriter(t *testing.T) {
startSerializedWriter(t)
db := initTrafficTestDB(t)
svc := &InboundService{}
clients := []model.Client{{Email: "depleted@x", ID: "aaaaaaaa-0000-0000-0000-000000000301", Enable: true}}
ib := mkInbound(t, 54102, model.VLESS, clientsSettings(t, clients))
if err := (&ClientService{}).SyncInbound(nil, ib.Id, clients); err != nil {
t.Fatalf("seed linkage: %v", err)
}
mkTraffic(t, ib.Id, "depleted@x", 60, 60, 100, 0, true)
release := parkSerializedWriter(t)
done := make(chan error, 1)
go func() { done <- svc.DelDepletedClients(-1) }()
queued := waitForQueuedWrite()
release()
if err := waitWriterResult(t, done); err != nil {
t.Fatalf("DelDepletedClients: %v", err)
}
if !queued {
t.Fatal("the depleted-client purge bypassed the serial traffic writer and opened its own transaction")
}
var inbounds int64
if err := db.Model(&model.Inbound{}).Where("id = ?", ib.Id).Count(&inbounds).Error; err != nil {
t.Fatalf("count inbounds: %v", err)
}
if inbounds != 0 {
t.Fatalf("depleted inbound %d still present after the purge", ib.Id)
}
var traffics int64
if err := db.Model(xray.ClientTraffic{}).Where("email = ?", "depleted@x").Count(&traffics).Error; err != nil {
t.Fatalf("count traffic rows: %v", err)
}
if traffics != 0 {
t.Fatalf("traffic rows for the depleted client = %d, want 0", traffics)
}
}