-
-
Notifications
You must be signed in to change notification settings - Fork 499
Description
Hi @sfackler, thanks again for all of your work on this library!
I believe I've found a bug in the Transaction
Drop implementation. Since the Tokio rewrite, the rollback that dropping an ongoing transaction is supposed to have is handled like:
rust-postgres/tokio-postgres/src/transaction.rs
Lines 51 to 54 in 4fd7527
let _ = self | |
.client | |
.inner() | |
.send(RequestMessages::Single(FrontendMessage::Raw(buf))); |
Notably, the response for the rollback is not actually polled in the Drop
method. The effect of this is that the rollback is not actually sent until the next time that a statement is awaited. But if no other statement is ever sent on this client, the transaction will not be rolled back. This is arguably expected when using tokio-postgres
because Drop
shouldn't block synchronously in an async event loop (or at least, I don't know how else that should work), but it's completely unexpected for non-async postgres
.
To demonstrate, make the following changes to the transaction_drop
test:
#[test]
fn transaction_drop() {
let mut client = Client::connect("host=localhost port=5433 user=postgres", NoTls).unwrap();
let mut client2 = Client::connect("host=localhost port=5433 user=postgres", NoTls).unwrap();
client
.simple_query("CREATE TABLE IF NOT EXISTS foo (id SERIAL PRIMARY KEY)")
.unwrap();
client
.execute("INSERT INTO foo VALUES (1) ON CONFLICT DO NOTHING", &[])
.unwrap();
let mut transaction = client.transaction().unwrap();
transaction
.execute("SELECT * FROM foo FOR UPDATE", &[])
.unwrap();
drop(transaction);
// Works if using client, blocks if using client2.
// let rows = client.query("SELECT * FROM foo FOR UPDATE", &[]).unwrap();
let rows = client2.query("SELECT * FROM foo FOR UPDATE", &[]).unwrap();
assert_eq!(rows.len(), 1);
}