Skip to content

Commit 7f56d6d

Browse files
committed
Error on leaked transaction handle
Treat a failed Arc::try_unwrap on the transaction handle as a transaction error instead of silently skipping rollback. database.rs now maps an unwrap failure to Error::transaction("transaction handle leaked outside the transaction scope") and still calls rollback when the unwrap succeeds. Added a test (sqlite_transaction_leak_on_error_returns_transaction_error) that leaks a transaction-scoped Database handle and asserts the new error is returned.
1 parent ca06ff2 commit 7f56d6d

2 files changed

Lines changed: 56 additions & 3 deletions

File tree

src/database.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -421,9 +421,12 @@ impl Database {
421421
Ok(result)
422422
}
423423
Err(e) => {
424-
if let Ok(txn) = Arc::try_unwrap(txn) {
425-
let _ = txn.rollback().await;
426-
}
424+
let txn = Arc::try_unwrap(txn).map_err(|_| {
425+
Error::transaction(
426+
"transaction handle leaked outside the transaction scope".to_string(),
427+
)
428+
})?;
429+
let _ = txn.rollback().await;
427430
Err(e)
428431
}
429432
}

tests/sqlite_ci_smoke_test.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1+
use std::sync::{Mutex, OnceLock};
2+
13
use tideorm::prelude::*;
24
use tideorm::{Database, TideConfig};
35

6+
fn leaked_transaction_db_slot() -> &'static Mutex<Option<Database>> {
7+
static LEAKED_TRANSACTION_DB: OnceLock<Mutex<Option<Database>>> = OnceLock::new();
8+
LEAKED_TRANSACTION_DB.get_or_init(|| Mutex::new(None))
9+
}
10+
411
#[derive(Model, PartialEq)]
512
#[tideorm(table = "ci_users")]
613
struct CiUser {
@@ -172,6 +179,49 @@ async fn sqlite_transaction_model_methods_use_transaction_connection() {
172179
assert_eq!(still_present.email, "baseline@example.com");
173180
}
174181

182+
#[tokio::test]
183+
async fn sqlite_transaction_leak_on_error_returns_transaction_error() {
184+
TideConfig::init()
185+
.database_type(DatabaseType::SQLite)
186+
.database("sqlite::memory:")
187+
.max_connections(1)
188+
.connect()
189+
.await
190+
.expect("failed to connect to SQLite");
191+
192+
leaked_transaction_db_slot()
193+
.lock()
194+
.expect("leaked transaction slot lock poisoned")
195+
.take();
196+
197+
let err = CiUser::transaction(|_tx| {
198+
Box::pin(async move {
199+
let leaked_db = tideorm::database::__current_db()
200+
.expect("transaction-scoped database should be available inside transaction");
201+
*leaked_transaction_db_slot()
202+
.lock()
203+
.expect("leaked transaction slot lock poisoned") = Some(leaked_db);
204+
205+
Err::<(), _>(tideorm::Error::query(
206+
"rollback with leaked transaction handle",
207+
))
208+
})
209+
})
210+
.await
211+
.expect_err("leaked transaction handle should surface as a transaction error");
212+
213+
assert!(
214+
err.to_string()
215+
.contains("transaction handle leaked outside the transaction scope"),
216+
"unexpected error: {err}"
217+
);
218+
219+
leaked_transaction_db_slot()
220+
.lock()
221+
.expect("leaked transaction slot lock poisoned")
222+
.take();
223+
}
224+
175225
#[tokio::test]
176226
async fn sqlite_query_with_and_find_with_work_without_global_db() {
177227
use tideorm::internal::{ActiveModelTrait, ConnectionTrait, InternalModel};

0 commit comments

Comments
 (0)