First of all, this plugin has been fantastic to work with so far. 💯
Here's a little bug I came across while working with begin_interruptible_transaction:
Currently, the drop handler for ActiveInterruptibleTransaction states:
|
// If writer is still present, it means commit/rollback wasn't called. |
|
// SQLite will automatically ROLLBACK the transaction when the connection |
|
// is returned to the pool if no explicit COMMIT was issued. |
|
if self.writer.is_some() { |
|
debug!( |
|
"Dropping transaction for db: {}, tx_id: {} (will auto-rollback)", |
|
self.db_path, self.transaction_id |
|
); |
|
} |
|
} |
IIUC, SQLite will automatically rollback the transaction when the connection is closed. SQLite has no concept of a connection pool, and it seems there's nothing elsewhere that closes the connection/rolls back the transaction when the write connection is returned to the pool (until there is a timeout). This leaves the write connection with an open transaction, and if some other code tries to open a transaction (reusing the write connection from the pool), it fails with a "Cannot start a transaction within a transaction" error.
There is also a bit of a mismatch between that code comment and the root README (which is technically accurate, I think):
|
* Always commit or rollback - abandoned transactions will rollback automatically |
|
on app exit |
Granted, our application code that uses these crates should catch errors and then roll back transactions intentionally. But it is easy to write code that looks like it's doing so but is actually open to this error.
For example, the code that surfaced this bug looks very similar to the README example:
|
### Interruptible Transactions (Rust) |
|
|
|
For transactions that need to read data mid-transaction: |
|
|
|
```rust |
|
// Assuming user_id, product_id, item_total are defined in your application context |
|
let user_id = 123; |
|
let product_id = 456; |
|
let item_total = 99.99; |
|
|
|
// Begin transaction with initial statements |
|
let mut tx = db.begin_interruptible_transaction() |
|
.execute(vec![ |
|
("INSERT INTO orders (user_id, total) VALUES (?, ?)", vec![json!(user_id), json!(0)]), |
|
]) |
|
.await?; |
|
|
|
// Read uncommitted data |
|
let orders = tx.read( |
|
"SELECT id FROM orders WHERE user_id = ? ORDER BY id DESC LIMIT 1".into(), |
|
vec![json!(user_id)] |
|
).await?; |
|
|
|
let order_id = orders[0].get("id").unwrap().as_i64().unwrap(); |
|
|
|
// Continue with more statements |
|
tx.continue_with(vec![ |
|
("INSERT INTO order_items (order_id, product_id) VALUES (?, ?)", vec![json!(order_id), json!(product_id)]), |
|
("UPDATE orders SET total = ? WHERE id = ?", vec![json!(item_total), json!(order_id)]), |
|
]).await?; |
|
|
|
// Commit (or rollback) |
|
tx.commit().await?; |
|
// tx.rollback().await?; // Alternative: rollback changes |
But all three of these innocuous-seeming ? on these lines:
create early returns if they fail for any reason. The early return drops the tx reference and leaves the transaction dangling.
For more info, I created this MCVE: https://github.com/yokuze/mcve/tree/main/cases/001-sqlx-sqlite-conn-mgr-interruptible-txns
You can run the example with:
git clone git@github.com:yokuze/mcve.git
cd mcve/cases/001-sqlx-sqlite-conn-mgr-interruptible-txns/
cargo run --bin sqlx-conn-mgr-txn-bug
Output:
[Step 1] Setup: create temp dir, connect, create test table
db path: /var/folders/np/aaaaaaaa/T/.bbbbbbb/test.db
table created
[Step 2] Start interruptible transaction, drop without commit/rollback
transaction started + INSERT executed
[LoggingTx::drop] dropping tx: step2
[Step 3] Probe writer pool state
acquire_writer returned in 90.833µs — pool likely considers writer IDLE/available
second begin_interruptible_transaction FAILED: error returned from database: (code: 1) cannot start a transaction within a transaction
[Step 4] Verify data state via read pool
row count: 0
[Step 5] Cleanup
db.remove() ok
=== Summary ===
BUG CONFIRMED: Dropping an interruptible transaction without commit/rollback
leaves the write connection with a dangling open transaction. The next caller
to acquire the writer gets a connection where BEGIN IMMEDIATE fails because
a transaction is already active.
First of all, this plugin has been fantastic to work with so far. 💯
Here's a little bug I came across while working with
begin_interruptible_transaction:Currently, the drop handler for
ActiveInterruptibleTransactionstates:tauri-plugin-sqlite/crates/sqlx-sqlite-toolkit/src/transactions.rs
Lines 235 to 244 in 240eb77
IIUC, SQLite will automatically rollback the transaction when the connection is closed. SQLite has no concept of a connection pool, and it seems there's nothing elsewhere that closes the connection/rolls back the transaction when the write connection is returned to the pool (until there is a timeout). This leaves the write connection with an open transaction, and if some other code tries to open a transaction (reusing the write connection from the pool), it fails with a "Cannot start a transaction within a transaction" error.
There is also a bit of a mismatch between that code comment and the root README (which is technically accurate, I think):
tauri-plugin-sqlite/README.md
Lines 384 to 385 in 240eb77
Granted, our application code that uses these crates should catch errors and then roll back transactions intentionally. But it is easy to write code that looks like it's doing so but is actually open to this error.
For example, the code that surfaced this bug looks very similar to the README example:
tauri-plugin-sqlite/README.md
Lines 783 to 816 in 240eb77
But all three of these innocuous-seeming
?on these lines:tauri-plugin-sqlite/README.md
Line 804 in 240eb77
tauri-plugin-sqlite/README.md
Line 812 in 240eb77
tauri-plugin-sqlite/README.md
Line 815 in 240eb77
create early returns if they fail for any reason. The early return drops the
txreference and leaves the transaction dangling.For more info, I created this MCVE: https://github.com/yokuze/mcve/tree/main/cases/001-sqlx-sqlite-conn-mgr-interruptible-txns
You can run the example with:
git clone git@github.com:yokuze/mcve.git cd mcve/cases/001-sqlx-sqlite-conn-mgr-interruptible-txns/ cargo run --bin sqlx-conn-mgr-txn-bugOutput: