Potentially inconsistent transaction state if COMMIT fails #2
Description
This is the same issue as mattn/go-sqlite3#184.
Quoting https://sqlite.org/lang_transaction.html#response_to_errors_within_a_transaction:
If certain kinds of errors occur within a transaction, the transaction may or may not be rolled back automatically.
The database/sql/driver.Tx interface effectively expects a single call to either Commit and Rollback to clean up the underlying connection in such a way that leaves it usable for new transactions (or possibly to leave it bricked in such a way that IsValid() will return false and thus the connection will be destroyed when returned to the pool, and in such a case the connection should probably also return errors if attempted to be used if acquired manually via (*sql.DB).Conn()).
Since sqlite's COMMIT may leave the connection in the transaction - but the sql.Tx machinery has no way to handle that - the implementation of (*sqlite.tx).Commit should also do a ROLLBACK in case of error, either unconditionally or maybe after checking sqlite3_get_autocommit. The former strategy is what mattn/go-sqlite3 ended up implementing in mattn/go-sqlite3#1071, I'm not sure if calling sqlite3_get_autocommit would be a little faster tho.
PoC for reproduction
package main
import (
"context"
"database/sql"
"errors"
"fmt"
"net/url"
"modernc.org/sqlite"
_ "modernc.org/sqlite"
sqlite3 "modernc.org/sqlite/lib"
)
func main() {
if err := run(context.Background()); err != nil {
panic(err)
}
}
func run(ctx context.Context) error {
db1, err := sql.Open("sqlite", (&url.URL{
Scheme: "file",
OmitHost: true,
Path: "./foo.db",
}).String())
if err != nil {
return err
}
defer db1.Close()
db1.SetMaxOpenConns(1)
db2, err := sql.Open("sqlite", (&url.URL{
Scheme: "file",
OmitHost: true,
Path: "./foo.db",
}).String())
if err != nil {
return err
}
defer db2.Close()
db2.SetMaxOpenConns(1)
if _, err := db1.ExecContext(ctx, "pragma busy_timeout = 1000"); err != nil {
return err
}
if _, err := db1.ExecContext(ctx, "drop table if exists foo; create table foo (bar integer primary key)"); err != nil {
return err
}
tx1, err := db1.BeginTx(ctx, nil)
if err != nil {
return err
}
defer tx1.Rollback()
tx2, err := db2.BeginTx(ctx, nil)
if err != nil {
return err
}
defer tx2.Rollback()
if _, err := tx1.ExecContext(ctx, "select * from foo"); err != nil {
return err
}
if _, err := tx2.ExecContext(ctx, "select * from foo"); err != nil {
return err
}
if _, err := tx1.ExecContext(ctx, "insert into foo (bar) values (42)"); err != nil {
return err
}
err = tx1.Commit()
if err == nil {
return fmt.Errorf("commit: expected error")
}
if sErr, ok := errors.AsType[*sqlite.Error](err); !ok || sErr.Code()&0xff != sqlite3.SQLITE_BUSY {
return fmt.Errorf("commit: expected sqlite error SQLITE_BUSY, got %w", err)
}
if err := tx1.Rollback(); err != sql.ErrTxDone {
return fmt.Errorf("commit: expected ErrTxDone, got %w", err)
}
tx3, err := db1.BeginTx(ctx, nil)
if err != nil {
return fmt.Errorf("we are now stuck because Rollback did nothing but the connection still has a transaction running: %w", err)
}
defer tx3.Rollback()
return nil
}