What the developer wrote
Authored by ziggie
graphdb: fix potential sql tx exhaustion
We should avoid taking the lock of a mutex inside transaction.
Currently we also take this lock in other places and there is a
chance that in case the application lock aquires the lock but
all transactions are already blocked waiting for the mutex to
unlock, we end up in a deadlock.
What changed, and why it matters
This commit fixes a deadlock risk in LND's channel graph database code. Previously, the code could acquire a database transaction first and then try to lock a cache mutex. Other code paths lock the cache mutex first and then take a database transaction. When two different parts of the program follow opposite lock ordering, they can get stuck waiting on each other forever (a deadlock). The fix makes both paths acquire the cache mutex before starting the database transaction, ensuring consistent ordering and preventing the deadlock. The commit title calls this 'potential sql tx exhaustion,' which suggests the deadlock could eventually exhaust available database transactions and freeze the service.
Treat this as a stability and availability fix. Review other graphdb code paths for the same inverted lock pattern, ensure all DB-bound cache accesses follow cacheMu-before-DB ordering, and consider adding static analysis or runtime lock-order detection to prevent regressions. No immediate cryptographic or remote exploit is evident, but operators should plan to upgrade to avoid potential node lockups.
Security signals we found
Deadlock / lock-order inversion between cache mutex and database transaction
Potential SQL transaction exhaustion as a consequence of deadlock
Denial-of-service vector against LND graph synchronization or gossip processing
Fix establishes consistent lock ordering (cacheMu -> DB)
Evidence from the diff
The patch changes fetchNextChanUpdateBatch in KVStore and ChanUpdatesInHorizon in SQLStore so that cacheMu (a shared read lock) is acquired before the database transaction is opened, held for the duration of the transaction, and released only after the transaction completes. Previously these functions opened a kvdb.View or ExecTx first and then took cacheMu.RLock() inside the transaction callback. This created inconsistent lock ordering: some paths take cacheMu then DB, while these paths took DB then cacheMu. Inverted lock ordering is a classic deadlock condition. The commit message explicitly states the deadlock scenario: if the application lock holds cacheMu while all transactions are blocked waiting for it, and the transaction-holding path is waiting for cacheMu, progress halts. The fix establishes a global cacheMu-before-DB ordering.
Changed components
graph/db/kv_store.go: fetchNextChanUpdateBatchgraph/db/sql_store.go: ChanUpdatesInHorizonShared channel cache mutex (cacheMu)KV/SQL database transaction layersInspect captured patch +18 / −8
diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go
index 5e94665..80e802e 100644
--- a/graph/db/kv_store.go
+++ b/graph/db/kv_store.go
@@ -2108,6 +2108,13 @@ func (c *KVStore) fetchNextChanUpdateBatch(
batch []ChannelEdge
hasMore bool
)
+
+ // Acquire read lock before starting transaction to ensure
+ // consistent lock ordering (cacheMu -> DB) and prevent
+ // deadlock with write operations.
+ c.cacheMu.RLock()
+ defer c.cacheMu.RUnlock()
+
err := kvdb.View(c.db, func(tx kvdb.RTx) error {
edges := tx.ReadBucket(edgeBucket)
if edges == nil {
@@ -2187,9 +2194,7 @@ func (c *KVStore) fetchNextChanUpdateBatch(
continue
}
- // Before we read the edge info, we'll see if this
- // element is already in the cache or not.
- c.cacheMu.RLock()
+ // Check cache (we already hold shared read lock).
if channel, ok := c.chanCache.get(chanIDInt); ok {
state.edgesSeen[chanIDInt] = struct{}{}
@@ -2200,11 +2205,8 @@ func (c *KVStore) fetchNextChanUpdateBatch(
indexKey, _ = updateCursor.Next()
- c.cacheMu.RUnlock()
-
continue
}
- c.cacheMu.RUnlock()
// The edge wasn't in the cache, so we'll fetch it along
// w/ the edge policies and nodes.
diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go
index 2bfcce5..3e2d74d 100644
--- a/graph/db/sql_store.go
+++ b/graph/db/sql_store.go
@@ -1127,6 +1127,11 @@ func (s *SQLStore) ChanUpdatesInHorizon(startTime, endTime time.Time,
for hasMore {
var batch []ChannelEdge
+ // Acquire read lock before starting transaction to
+ // ensure consistent lock ordering (cacheMu -> DB) and
+ // prevent deadlock with write operations.
+ s.cacheMu.RLock()
+
err := s.db.ExecTx(ctx, sqldb.ReadTxOpt(),
func(db SQLQueries) error {
//nolint:ll
@@ -1179,11 +1184,11 @@ func (s *SQLStore) ChanUpdatesInHorizon(startTime, endTime time.Time,
continue
}
- s.cacheMu.RLock()
+ // Check cache (we already hold
+ // shared read lock).
channel, ok := s.chanCache.get(
chanIDInt,
)
- s.cacheMu.RUnlock()
if ok {
hits++
total++
@@ -1217,6 +1222,9 @@ func (s *SQLStore) ChanUpdatesInHorizon(startTime, endTime time.Time,
)
})
+ // Release read lock after transaction completes.
+ s.cacheMu.RUnlock()
+
if err != nil {
log.Errorf("ChanUpdatesInHorizon "+
"batch error: %v", err)
Why this scored 60/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.