diff --git a/rbf/db.go b/rbf/db.go index cc4b65700..99c4f0e2f 100644 --- a/rbf/db.go +++ b/rbf/db.go @@ -444,15 +444,10 @@ func (db *DB) Begin(writable bool) (_ *Tx, err error) { } db.mu.Lock() - // note: We cannot defer db.mu.Unlock() here because - // we call tx.Rollback() before if db.readMetaPage - // returns an error, and thus we will deadlock against - // ourselves when the Rollback tries to acquire the db.mu. - // This is why db.mu.Unlock() is done manually below. + defer db.mu.Unlock() if !db.opened { cleanup() - db.mu.Unlock() return nil, ErrClosed } @@ -470,6 +465,11 @@ func (db *DB) Begin(writable bool) (_ *Tx, err error) { DeleteEmptyContainer: true, } + defer func() { + if err != nil { + tx.rollback(true) + } + }() if writable { tx.dirtyPages = make(map[uint32][]byte) @@ -480,10 +480,6 @@ func (db *DB) Begin(writable bool) (_ *Tx, err error) { // This page is only written at the end of a dirty transaction. page, err := db.readMetaPage() if err != nil { - // we will deadlock in tx.Rollback() - // on db.mu.Lock unless we manually db.mu.Unlock first. - db.mu.Unlock() - tx.Rollback() return nil, err } copy(tx.meta[:], page) @@ -499,13 +495,10 @@ func (db *DB) Begin(writable bool) (_ *Tx, err error) { // this avoids recomputing the cache if there are no write txs for a while. if db.rootRecords == nil { if db.rootRecords, err = tx.RootRecords(); err != nil { - db.mu.Unlock() - tx.Rollback() return nil, err } } - db.mu.Unlock() return tx, nil } diff --git a/rbf/tx.go b/rbf/tx.go index 37c76a351..38fed48f6 100644 --- a/rbf/tx.go +++ b/rbf/tx.go @@ -126,7 +126,9 @@ func (tx *Tx) Commit() error { return tx.db.removeTx(tx) } -func (tx *Tx) Rollback() { +func (tx *Tx) Rollback() { tx.rollback(false) } + +func (tx *Tx) rollback(hasDBLock bool) { tx.mu.Lock() defer tx.mu.Unlock() @@ -141,8 +143,10 @@ func (tx *Tx) Rollback() { } // Disconnect transaction from DB. - tx.db.mu.Lock() - defer tx.db.mu.Unlock() + if !hasDBLock { + tx.db.mu.Lock() + defer tx.db.mu.Unlock() + } vprint.PanicOn(tx.db.removeTx(tx)) }