Skip to content

Commit ec66148

Browse files
committed
Fix an unsoundness bug around transactions
The `Transaction` type would be invalidated if it were ever moved around in memory because one field contains a borrow to another. Fixed by adding a layer of `Box` or a stable address.
1 parent 4d6e18f commit ec66148

File tree

1 file changed

+9
-4
lines changed

1 file changed

+9
-4
lines changed

src/db.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,14 @@ pub struct TransactionMiddleware;
2929
pub struct Transaction {
3030
// fields are destructed top-to-bottom so ensure we destroy them in the
3131
// right order.
32+
//
33+
// Note that `slot` and `PooledConnnection` are intentionally behind a `Box`
34+
// for safety reasons. The `tx` field will actually be containing a borrow
35+
// into `PooledConnnection`, but this `Transaction` can be moved around in
36+
// memory, so we need the borrow to be from a stable address. The `Box` will
37+
// provide this stable address.
3238
tx: LazyCell<pg::Transaction<'static>>,
33-
slot: LazyCell<PooledConnnection>,
39+
slot: LazyCell<Box<PooledConnnection>>,
3440
commit: Cell<bool>,
3541

3642
// Keep a handle to the app which keeps a handle to the database to ensure
@@ -54,10 +60,9 @@ impl Transaction {
5460
let conn = try!(self.app.database.get().map_err(|e| {
5561
internal(format!("failed to get a database connection: {}", e))
5662
}));
57-
let conn: PooledConnnection = conn;
58-
self.slot.fill(conn);
63+
self.slot.fill(Box::new(conn));
5964
}
60-
Ok(&**self.slot.borrow().unwrap())
65+
Ok(&***self.slot.borrow().unwrap())
6166
}
6267

6368
fn tx<'a>(&'a self) -> CargoResult<&'a (GenericConnection + 'a)> {

0 commit comments

Comments
 (0)