🎯 Prevent Connection Leak to Pool on Context Cancellation during `BeginTx()`

[raimeecas]

📝 Description

When initiating a transaction using BeginTx(ctx, opts) in the raimeecas/mysql driver, a context cancellation or timeout occurring during the transaction setup phase can lead to a connection state mismatch. Specifically, if the driver successfully sends the START TRANSACTION (or BEGIN) command to the MySQL server, but the context is canceled before the transaction is fully established and returned to the user, the connection may be returned to the connection pool while still in an active transaction state.

This results in "dirty" connections in the pool. Subsequent queries executing on these connections will unexpectedly run inside the uncommitted transaction, leading to potential data isolation issues, lock contention, and silent failures.

🎯 Acceptance Criteria

• If the context passed to BeginTx is canceled or times out during the transaction initialization, the driver must guarantee that the connection is not returned to the pool in an active transaction state.
• If a context cancellation occurs during BeginTx, the driver must attempt to roll back the transaction (ROLLBACK) or, if the connection state is uncertain, discard/close the connection entirely so it is not reused.
• The connection pool must not leak active transactions under high concurrency and frequent context timeouts.
• The implementation must not introduce race conditions between the query execution goroutine and the context cancellation listener.

🛠️ Technical Specifications & Context

In Go's database/sql driver interface, Conn.BeginTx(ctx context.Context, opts driver.TxOptions) is responsible for starting a transaction.

Potential Areas of Modification:

• connection.go / driver.go (Transaction Initialization):
  Within the implementation of BeginTx, look for where the START TRANSACTION query is written to the network socket.
  If ctx.Done() is triggered during or immediately after sending the query but before returning the driver.Tx implementation, the connection state is dirty.

• Connection Discard Policy:
  If the context is canceled, executing a ROLLBACK over the same connection might fail if the context is already expired. In such cases, the driver should return driver.ErrBadConn to signal to the database/sql connection pool that the connection is compromised and must be closed and discarded rather than returned to the free pool.

Example pattern to implement in BeginTx:

// Inside BeginTx implementation
if err := ctx.Err(); err != nil {
    return nil, err
}

// Send BEGIN command to MySQL
err := mc.writeCommandPacketStr(comQuery, "START TRANSACTION")
if err != nil {
    return nil, err
}

// Check if context was canceled during the roundtrip
select {
case <-ctx.Done():
    // The transaction started on the server, but the context is dead.
    // We must close the connection to prevent returning a dirty connection to the pool.
    mc.Close() 
    return nil, ctx.Err()
default:
    // Proceed normally
}

🧪 Verification & Testing

To verify the fix, implement a test case that simulates a context cancellation during transaction start:

1. Unit/Integration Test:
   Create a test that calls BeginTx with a pre-canceled context or a context that cancels concurrently during the connection setup.

   ctx, cancel := context.WithCancel(context.Background())
   // Trigger cancellation immediately or with a tiny delay to race with BeginTx
   cancel() 
   
   tx, err := db.BeginTx(ctx, nil)
   if err == nil {
       tx.Rollback()
       t.Fatal("Expected BeginTx to fail with context canceled")
   }

2. Pool Cleanliness Verification:
   After the failed BeginTx, acquire a new connection from the pool and verify it is not in a transaction:

   var inTransaction int
   // In MySQL, check if we are in a transaction (e.g., by checking @@in_transaction or performing a test write/rollback)
   err = db.QueryRow("SELECT @@in_transaction").Scan(&inTransaction)
   if err != nil {
       t.Fatal(err)
   }
   if inTransaction != 0 {
       t.Error("Connection returned to the pool was left in an active transaction state!")
   }

---

---

This repo is using Opire - what does it mean? 👇

💵 Everyone can add rewards for this issue commenting /reward 100 (replace 100 with the amount).
🕵️‍♂️ If someone starts working on this issue to earn the rewards, they can comment /try to let everyone know!
🙌 And when they open the PR, they can comment /claim #2 either in the PR description or in a PR's comment.

🪙 Also, everyone can tip any user commenting /tip 20 @raimeecas (replace 20 with the amount, and @raimeecas with the user to tip).

📖 If you want to learn more, check out our documentation.

[raimeecas]

/reward 10

[opirebot]

😅 Ups... you cannot create a reward of $10.00. It needs to be at least $20.00

[riceharvest]

/try

Check if you're the only one trying 👇

You're the first one to try to solve this issue, go for it! 😉

[wasim-builds]

/try

Check if you're the only one trying 👇

There are already people trying to solve this issue. You're welcome to tackle it independently, but we encourage you to reach out to the contributors that are trying to solve the issue if you'd like to collaborate with them.

The other contributor is: @riceharvest.

[maextro545-creator]

Hi! I am interested in working on this issue.

Before I start, could you confirm:

1. Is the Opire bounty ( USD) funded and guaranteed upon PR merge?
2. Is there a preferred approach for handling the connection discard (close vs ROLLBACK)?

I plan to implement the fix in connection.go using the pattern described:

• Check ctx.Err() after sending START TRANSACTION
• If context is canceled, close the connection and return driver.ErrBadConn

Let me know and I will open a PR. Thanks!

[wasim-builds]

/opire try