From 4a7c96f533e358e6183448a6b190a3f858a55246 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 27 Sep 2026 02:34:39 +0000 Subject: [PATCH] fix(advisorylock): bound HeldLock.TryAcquire on a hung database The failover prober gate calls TryAcquire with the application context, which never ends. A database that stopped answering blocked the scheduler goroutine for good, with the lock's mutex held. Bound the session open and the lock query with the same timeout as Verify. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Ettore Di Giacinto --- core/services/advisorylock/held_lock.go | 13 +++- .../advisorylock/held_lock_timeout_test.go | 60 +++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 core/services/advisorylock/held_lock_timeout_test.go diff --git a/core/services/advisorylock/held_lock.go b/core/services/advisorylock/held_lock.go index 46f2db6f5..99ece9e22 100644 --- a/core/services/advisorylock/held_lock.go +++ b/core/services/advisorylock/held_lock.go @@ -12,9 +12,10 @@ import ( "gorm.io/gorm" ) -// heldLockCheckTimeout bounds the liveness check and the unlock, so a hung -// database cannot stall the caller's loop. -const heldLockCheckTimeout = 5 * time.Second +// heldLockCheckTimeout bounds an acquire, the liveness check and the unlock, +// so a hung database cannot stall the caller's loop. A var so tests can +// shorten it. +var heldLockCheckTimeout = 5 * time.Second // heldLockSessionSettings make the server notice a lock holder whose host // died without closing the connection (crash, power loss, partition) within @@ -80,6 +81,12 @@ func (l *HeldLock) TryAcquire(ctx context.Context) (bool, error) { } } + // Callers pass long-lived contexts (the application's, which never ends), + // and l.mu is held throughout: a hung database would otherwise block this + // caller and every other user of the lock for good. The deadline only + // covers obtaining the session, which stays usable afterwards. + ctx, cancel := context.WithTimeout(ctx, heldLockCheckTimeout) + defer cancel() if l.conn == nil { conn, err := l.openSession(ctx) if err != nil { diff --git a/core/services/advisorylock/held_lock_timeout_test.go b/core/services/advisorylock/held_lock_timeout_test.go new file mode 100644 index 000000000..0f8c6f0a3 --- /dev/null +++ b/core/services/advisorylock/held_lock_timeout_test.go @@ -0,0 +1,60 @@ +package advisorylock + +import ( + "context" + "database/sql" + "database/sql/driver" + "errors" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "gorm.io/driver/postgres" + "gorm.io/gorm" +) + +// hungConnector opens connections that accept session settings but never +// answer a query, like a database host that stopped responding mid-session. +type hungConnector struct{} + +func (hungConnector) Connect(context.Context) (driver.Conn, error) { return hungConn{}, nil } +func (hungConnector) Driver() driver.Driver { return hungDriver{} } + +type hungDriver struct{} + +func (hungDriver) Open(string) (driver.Conn, error) { return hungConn{}, nil } + +type hungConn struct{} + +func (hungConn) Prepare(string) (driver.Stmt, error) { return nil, errors.New("not supported") } +func (hungConn) Close() error { return nil } +func (hungConn) Begin() (driver.Tx, error) { return nil, errors.New("not supported") } +func (hungConn) ExecContext(context.Context, string, []driver.NamedValue) (driver.Result, error) { + return driver.RowsAffected(0), nil +} +func (hungConn) QueryContext(ctx context.Context, _ string, _ []driver.NamedValue) (driver.Rows, error) { + <-ctx.Done() + return nil, ctx.Err() +} + +var _ = Describe("HeldLock against a hung database", func() { + It("gives up an acquire after a bounded time, even with a context that never ends", func() { + prev := heldLockCheckTimeout + heldLockCheckTimeout = 200 * time.Millisecond + DeferCleanup(func() { heldLockCheckTimeout = prev }) + + db, err := gorm.Open(postgres.New(postgres.Config{Conn: sql.OpenDB(hungConnector{})}), &gorm.Config{DisableAutomaticPing: true}) + Expect(err).ToNot(HaveOccurred()) + l := NewHeldLock(db, 4242) + + done := make(chan error, 1) + go func() { + _, err := l.TryAcquire(context.Background()) + done <- err + }() + var got error + Eventually(done, 5*time.Second).Should(Receive(&got)) + Expect(got).To(HaveOccurred()) + Expect(l.Held()).To(BeFalse()) + }) +})