fix locking and list management for afterCurrentTx

Two issues: First, there was a race condition because we were never
using the mutex for anything but the condvar broadcast, second, there
was no reason for the afterCurrentTx to need to maintain the list since
we already know where in the list we are when we are waking it up.

afterCurrentTx still wants to run with the db lock held, because
the degenerate case (no outstanding Tx) means that it will be running
with it held already. That's for another commit.

rbf/db.go

@@ -676,15 +676,6 @@ func (db *DB) afterCurrentTx(callback func()) {
 		// fmt.Printf("afterCurrentTx: locking db\n")
 		db.mu.Lock()
 		defer db.mu.Unlock()
-		// remove us from the db's list
-		for i, v := range db.txWaiters {
-			if v == txw {
-				// remove us from the list
-				copy(db.txWaiters[i:], db.txWaiters[i+1:])
-				db.txWaiters = db.txWaiters[:len(db.txWaiters)-1]
-				break
-			}
-		}
 		// fmt.Printf("afterCurrentTx: running callback\n")
 		txw.callback()
 	}()
@@ -716,12 +707,23 @@ func (db *DB) removeTx(tx *Tx) error {
 	}
 	// remove ourselves from the list of transactions the db is keeping.
 	delete(tx.db.txs, tx)
-	for _, txw := range tx.db.txWaiters {
+	for i := 0; i < len(tx.db.txWaiters); i++ {
+		txw := tx.db.txWaiters[i]
+		// in practice this probably never matters, but theoretically the
+		// goroutine that's waiting on the condition variable may
+		// not have performed its first test on len(txw.waitingOn) yet.
+		txw.mu.Lock()
 		delete(txw.waitingOn, tx)
+		txw.mu.Unlock()
 		// let it know we're done. we've still got db.mu.lock, so it won't
 		// happen just yet, but it'll be able to continue.
 		if len(txw.waitingOn) == 0 {
+			// remove us from the db's list
+			copy(db.txWaiters[i:], db.txWaiters[i+1:])
+			db.txWaiters = db.txWaiters[:len(db.txWaiters)-1]
 			txw.cond.Broadcast()
+			// decrement i so we don't skip an entry we just copied in to [i]
+			i--
 		}
 	}