mirror of
https://github.com/Sagit-chu/flvx.git
synced 2026-10-06 01:56:37 +08:00
fix(backend): resolve SQLite deadlock in tunnel entry updates (#334)
- Fix deadlock when updating tunnel entries with offline nodes - Add test file for tunnel entry SQLite operations - Update contract tests for entry port conflict and limiter sync - Add plan documents for SQLite deadlock fix and contract semantics
This commit is contained in:
@@ -832,7 +832,7 @@ func (h *Handler) tunnelUpdate(w http.ResponseWriter, r *http.Request) {
|
||||
newEntryNodeIDs = append(newEntryNodeIDs, in.NodeID)
|
||||
}
|
||||
}
|
||||
if err := h.validateTunnelEntryPortConflictsForNewEntries(id, oldEntryNodeIDs, newEntryNodeIDs); err != nil {
|
||||
if err := h.validateTunnelEntryPortConflictsForNewEntries(tx, id, oldEntryNodeIDs, newEntryNodeIDs); err != nil {
|
||||
response.WriteJSON(w, response.ErrDefault(err.Error()))
|
||||
return
|
||||
}
|
||||
@@ -1020,8 +1020,8 @@ func (h *Handler) cleanupTunnelForwardRuntimesOnRemovedEntryNodes(tunnelID int64
|
||||
}
|
||||
}
|
||||
|
||||
func (h *Handler) validateTunnelEntryPortConflictsForNewEntries(tunnelID int64, oldEntryNodeIDs, newEntryNodeIDs []int64) error {
|
||||
if h == nil || h.repo == nil || tunnelID <= 0 {
|
||||
func (h *Handler) validateTunnelEntryPortConflictsForNewEntries(tx *gorm.DB, tunnelID int64, oldEntryNodeIDs, newEntryNodeIDs []int64) error {
|
||||
if h == nil || h.repo == nil || tx == nil || tunnelID <= 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -1030,7 +1030,7 @@ func (h *Handler) validateTunnelEntryPortConflictsForNewEntries(tunnelID int64,
|
||||
return nil
|
||||
}
|
||||
|
||||
forwards, err := h.listForwardsByTunnel(tunnelID)
|
||||
forwards, err := h.repo.ListForwardsByTunnelTx(tx, tunnelID)
|
||||
if err != nil || len(forwards) == 0 {
|
||||
return nil
|
||||
}
|
||||
@@ -1040,7 +1040,7 @@ func (h *Handler) validateTunnelEntryPortConflictsForNewEntries(tunnelID int64,
|
||||
if f == nil {
|
||||
continue
|
||||
}
|
||||
oldPorts, portsErr := h.listForwardPorts(f.ID)
|
||||
oldPorts, portsErr := h.repo.ListForwardPortsTx(tx, f.ID)
|
||||
if portsErr != nil {
|
||||
continue
|
||||
}
|
||||
@@ -1050,14 +1050,14 @@ func (h *Handler) validateTunnelEntryPortConflictsForNewEntries(tunnelID int64,
|
||||
}
|
||||
|
||||
for _, nodeID := range addedNodeIDs {
|
||||
node, nodeErr := h.getNodeRecord(nodeID)
|
||||
node, nodeErr := h.repo.GetNodeRecordTx(tx, nodeID)
|
||||
if nodeErr != nil {
|
||||
continue
|
||||
}
|
||||
if err := validateLocalNodePort(node, port); err != nil {
|
||||
return fmt.Errorf("转发 %s 入口端口冲突: %w", f.Name, err)
|
||||
}
|
||||
if err := h.validateForwardPortAvailability(node, port, f.ID); err != nil {
|
||||
if err := h.validateForwardPortAvailabilityTx(tx, node, port, f.ID); err != nil {
|
||||
return fmt.Errorf("转发 %s 入口端口冲突: %w", f.Name, err)
|
||||
}
|
||||
}
|
||||
@@ -4153,6 +4153,20 @@ func (h *Handler) validateForwardPortAvailability(node *nodeRecord, port int, cu
|
||||
return nil
|
||||
}
|
||||
|
||||
func (h *Handler) validateForwardPortAvailabilityTx(tx *gorm.DB, node *nodeRecord, port int, currentForwardID int64) error {
|
||||
if h == nil || h.repo == nil || tx == nil || node == nil || port <= 0 {
|
||||
return nil
|
||||
}
|
||||
occupied, err := h.repo.HasOtherForwardOnNodePortTx(tx, node.ID, port, currentForwardID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if occupied {
|
||||
return fmt.Errorf("节点 %s 端口 %d 已被其他转发占用", node.Name, port)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func parsePortRangeMinMax(input string) (int, int) {
|
||||
input = strings.TrimSpace(input)
|
||||
if input == "" {
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"go-backend/internal/store/repo"
|
||||
)
|
||||
|
||||
func TestValidateTunnelEntryPortConflictsForNewEntriesDoesNotBlockOnSQLiteTx(t *testing.T) {
|
||||
r, err := repo.Open(filepath.Join(t.TempDir(), "panel.db"))
|
||||
if err != nil {
|
||||
t.Fatalf("open sqlite: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
_ = r.Close()
|
||||
})
|
||||
h := &Handler{repo: r}
|
||||
now := time.Now().UnixMilli()
|
||||
|
||||
if err := r.DB().Exec(`
|
||||
INSERT INTO node(name, secret, server_ip, port, created_time, status, tcp_listen_addr, udp_listen_addr, is_remote)
|
||||
VALUES
|
||||
('entry-old', 'secret-old', '10.0.0.1', '12000-12010', ?, 1, '[::]', '[::]', 0),
|
||||
('entry-new', 'secret-new', '10.0.0.2', '12000-12010', ?, 1, '[::]', '[::]', 0)
|
||||
`, now, now).Error; err != nil {
|
||||
t.Fatalf("insert nodes: %v", err)
|
||||
}
|
||||
var oldEntryID, newEntryID int64
|
||||
if err := r.DB().Raw(`SELECT id FROM node WHERE name = 'entry-old'`).Scan(&oldEntryID).Error; err != nil {
|
||||
t.Fatalf("load old entry id: %v", err)
|
||||
}
|
||||
if err := r.DB().Raw(`SELECT id FROM node WHERE name = 'entry-new'`).Scan(&newEntryID).Error; err != nil {
|
||||
t.Fatalf("load new entry id: %v", err)
|
||||
}
|
||||
|
||||
if err := r.DB().Exec(`
|
||||
INSERT INTO tunnel(name, traffic_ratio, type, protocol, flow, created_time, updated_time, status, inx, ip_preference)
|
||||
VALUES('sqlite-tunnel', 1, 1, 'tls', 1, ?, ?, 1, 1, '')
|
||||
`, now, now).Error; err != nil {
|
||||
t.Fatalf("insert tunnel: %v", err)
|
||||
}
|
||||
var tunnelID int64
|
||||
if err := r.DB().Raw(`SELECT id FROM tunnel WHERE name = 'sqlite-tunnel'`).Scan(&tunnelID).Error; err != nil {
|
||||
t.Fatalf("load tunnel id: %v", err)
|
||||
}
|
||||
|
||||
if err := r.DB().Exec(`
|
||||
INSERT INTO chain_tunnel(tunnel_id, chain_type, node_id, inx, protocol)
|
||||
VALUES(?, '1', ?, 1, 'tls')
|
||||
`, tunnelID, oldEntryID).Error; err != nil {
|
||||
t.Fatalf("insert chain_tunnel: %v", err)
|
||||
}
|
||||
|
||||
if err := r.DB().Exec(`
|
||||
INSERT INTO forward(user_id, user_name, name, tunnel_id, remote_addr, strategy, created_time, updated_time, status, inx)
|
||||
VALUES(1, 'tester', 'forward-a', ?, '127.0.0.1:8080', 'fifo', ?, ?, 1, 1)
|
||||
`, tunnelID, now, now).Error; err != nil {
|
||||
t.Fatalf("insert forward: %v", err)
|
||||
}
|
||||
var forwardID int64
|
||||
if err := r.DB().Raw(`SELECT id FROM forward WHERE name = 'forward-a'`).Scan(&forwardID).Error; err != nil {
|
||||
t.Fatalf("load forward id: %v", err)
|
||||
}
|
||||
|
||||
if err := r.DB().Exec(`
|
||||
INSERT INTO forward_port(forward_id, node_id, port)
|
||||
VALUES(?, ?, 12001)
|
||||
`, forwardID, oldEntryID).Error; err != nil {
|
||||
t.Fatalf("insert forward_port: %v", err)
|
||||
}
|
||||
|
||||
tx := r.BeginTx()
|
||||
if tx == nil {
|
||||
t.Fatal("begin tx: nil transaction")
|
||||
}
|
||||
if tx.Error != nil {
|
||||
t.Fatalf("begin tx: %v", tx.Error)
|
||||
}
|
||||
|
||||
errCh := make(chan error, 1)
|
||||
doneCh := make(chan struct{})
|
||||
go func() {
|
||||
defer close(doneCh)
|
||||
errCh <- h.validateTunnelEntryPortConflictsForNewEntries(tx, tunnelID, []int64{oldEntryID}, []int64{oldEntryID, newEntryID})
|
||||
}()
|
||||
|
||||
select {
|
||||
case err := <-errCh:
|
||||
if err != nil {
|
||||
_ = tx.Rollback().Error
|
||||
t.Fatalf("unexpected validation error: %v", err)
|
||||
}
|
||||
case <-time.After(500 * time.Millisecond):
|
||||
_ = tx.Rollback().Error
|
||||
<-doneCh
|
||||
t.Fatal("validation blocked while transaction was open on sqlite")
|
||||
}
|
||||
|
||||
if err := tx.Rollback().Error; err != nil {
|
||||
t.Fatalf("rollback tx: %v", err)
|
||||
}
|
||||
}
|
||||
@@ -30,8 +30,15 @@ func (r *Repository) ListForwardsByTunnel(tunnelID int64) ([]model.ForwardRecord
|
||||
if r == nil || r.db == nil {
|
||||
return nil, errors.New("repository not initialized")
|
||||
}
|
||||
return r.ListForwardsByTunnelTx(r.db, tunnelID)
|
||||
}
|
||||
|
||||
func (r *Repository) ListForwardsByTunnelTx(tx *gorm.DB, tunnelID int64) ([]model.ForwardRecord, error) {
|
||||
if tx == nil {
|
||||
return nil, errors.New("database unavailable")
|
||||
}
|
||||
var forwards []model.Forward
|
||||
err := r.db.Where("tunnel_id = ?", tunnelID).Order("id ASC").Find(&forwards).Error
|
||||
err := tx.Where("tunnel_id = ?", tunnelID).Order("id ASC").Find(&forwards).Error
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -95,8 +102,15 @@ func (r *Repository) ListForwardPorts(forwardID int64) ([]model.ForwardPortRecor
|
||||
if r == nil || r.db == nil {
|
||||
return nil, errors.New("repository not initialized")
|
||||
}
|
||||
return r.ListForwardPortsTx(r.db, forwardID)
|
||||
}
|
||||
|
||||
func (r *Repository) ListForwardPortsTx(tx *gorm.DB, forwardID int64) ([]model.ForwardPortRecord, error) {
|
||||
if tx == nil {
|
||||
return nil, errors.New("database unavailable")
|
||||
}
|
||||
var ports []model.ForwardPort
|
||||
err := r.db.Where("forward_id = ?", forwardID).Order("id ASC").Find(&ports).Error
|
||||
err := tx.Where("forward_id = ?", forwardID).Order("id ASC").Find(&ports).Error
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -115,12 +129,19 @@ func (r *Repository) HasOtherForwardOnNodePort(nodeID int64, port int, currentFo
|
||||
if r == nil || r.db == nil {
|
||||
return false, errors.New("repository not initialized")
|
||||
}
|
||||
return r.HasOtherForwardOnNodePortTx(r.db, nodeID, port, currentForwardID)
|
||||
}
|
||||
|
||||
func (r *Repository) HasOtherForwardOnNodePortTx(tx *gorm.DB, nodeID int64, port int, currentForwardID int64) (bool, error) {
|
||||
if tx == nil {
|
||||
return false, errors.New("database unavailable")
|
||||
}
|
||||
if nodeID <= 0 || port <= 0 {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
var count int64
|
||||
err := r.db.Model(&model.ForwardPort{}).
|
||||
err := tx.Model(&model.ForwardPort{}).
|
||||
Where("node_id = ? AND port = ? AND forward_id <> ?", nodeID, port, currentForwardID).
|
||||
Count(&count).Error
|
||||
if err != nil {
|
||||
|
||||
@@ -32,7 +32,6 @@ func TestIssue313_EntryPortCrossTunnelConflictContract(t *testing.T) {
|
||||
return mustLastInsertID(t, repo, name)
|
||||
}
|
||||
|
||||
entryA := insertNode("issue313-entry-a", "10.100.0.1", "2000-2010")
|
||||
entryB1 := insertNode("issue313-entry-b1", "10.100.0.2", "2000-2010")
|
||||
entryB2 := insertNode("issue313-entry-b2", "10.100.0.3", "2000-2010")
|
||||
chainA := insertNode("issue313-chain-a", "10.100.0.4", "3000-3010")
|
||||
@@ -51,7 +50,7 @@ func TestIssue313_EntryPortCrossTunnelConflictContract(t *testing.T) {
|
||||
if err := repo.DB().Exec(`
|
||||
INSERT INTO chain_tunnel(tunnel_id, chain_type, node_id, port, strategy, inx, protocol)
|
||||
VALUES(?, 1, ?, 2000, 'round', 1, 'tls')
|
||||
`, tunnelAID, entryA).Error; err != nil {
|
||||
`, tunnelAID, entryB2).Error; err != nil {
|
||||
t.Fatalf("insert chain_tunnel entry a: %v", err)
|
||||
}
|
||||
if err := repo.DB().Exec(`
|
||||
@@ -109,7 +108,7 @@ func TestIssue313_EntryPortCrossTunnelConflictContract(t *testing.T) {
|
||||
}
|
||||
forwardAID := mustLastInsertID(t, repo, "issue313-forward-a")
|
||||
|
||||
if err := repo.DB().Exec(`INSERT INTO forward_port(forward_id, node_id, port) VALUES(?, ?, ?)`, forwardAID, entryA, 2000).Error; err != nil {
|
||||
if err := repo.DB().Exec(`INSERT INTO forward_port(forward_id, node_id, port) VALUES(?, ?, ?)`, forwardAID, entryB2, 2000).Error; err != nil {
|
||||
t.Fatalf("insert forward_port a: %v", err)
|
||||
}
|
||||
|
||||
@@ -171,7 +170,7 @@ func TestIssue313_EntryPortCrossTunnelConflictContract(t *testing.T) {
|
||||
t.Fatalf("expected update failure due to cross-tunnel port conflict, got success with code 0")
|
||||
}
|
||||
|
||||
if !bytes.Contains(res.Body.Bytes(), []byte("端口")) && !bytes.Contains(res.Body.Bytes(), []byte("占用")) {
|
||||
if !bytes.Contains([]byte(out.Msg), []byte("端口")) && !bytes.Contains([]byte(out.Msg), []byte("占用")) {
|
||||
t.Fatalf("expected port conflict error message, got %q", out.Msg)
|
||||
}
|
||||
|
||||
|
||||
@@ -705,7 +705,7 @@ func TestTunnelUpdateChangesEntryNodeButLeavesOldForwardRuntimeContract(t *testi
|
||||
}
|
||||
|
||||
oldEntryNodeID := insertNode("issue281-old-entry", "issue281-old-entry-secret", "10.51.0.1", "51000-51010", 0)
|
||||
newEntryNodeID := insertNode("issue281-new-entry", "issue281-new-entry-secret", "10.51.0.2", "52000-52010", 1)
|
||||
newEntryNodeID := insertNode("issue281-new-entry", "issue281-new-entry-secret", "10.51.0.2", "51000-51010", 1)
|
||||
exitNodeID := insertNode("issue281-exit", "issue281-exit-secret", "10.51.0.3", "53000-53010", 2)
|
||||
|
||||
if err := r.DB().Exec(`
|
||||
@@ -881,8 +881,8 @@ func TestTunnelUpdateEntryTransitionsCleanupForwardRuntimeContract(t *testing.T)
|
||||
}
|
||||
|
||||
entryA := insertNode("issue281-transition-entry-a", "issue281-transition-entry-a-secret", "10.52.0.1", "54000-54010", 0)
|
||||
entryB := insertNode("issue281-transition-entry-b", "issue281-transition-entry-b-secret", "10.52.0.2", "55000-55010", 1)
|
||||
entryC := insertNode("issue281-transition-entry-c", "issue281-transition-entry-c-secret", "10.52.0.3", "56000-56010", 2)
|
||||
entryB := insertNode("issue281-transition-entry-b", "issue281-transition-entry-b-secret", "10.52.0.2", "54000-54010", 1)
|
||||
entryC := insertNode("issue281-transition-entry-c", "issue281-transition-entry-c-secret", "10.52.0.3", "54000-54010", 2)
|
||||
exitNodeID := insertNode("issue281-transition-exit", "issue281-transition-exit-secret", "10.52.0.4", "57000-57010", 3)
|
||||
|
||||
if err := r.DB().Exec(`
|
||||
|
||||
Reference in New Issue
Block a user