Implement Deref to ReadTransaction for WriteTransaction

1

I'm trying to implement Implement Deref to ReadTransaction for the write Transaction type in possum-db: possum/src/tx.rs at 7d660fa365f8d0b4e61915eb109fc9690a2c810d · anacrolix/possum · GitHub

This gives me

error: lifetime may not live long enough
   --> src/tx.rs:181:13
    |
176 |   impl<'h> Deref for Transaction<'h> {
    |        -- lifetime `'h` defined here
...
179 |       fn deref(&self) -> &Self::Target {
    |                - let's call the lifetime of this reference `'1`
180 |           unsafe {
181 | /             &ReadTransaction {
182 | |                 tx: ReadOnlyRusqliteTransaction { conn: &self.tx },
183 | |             }
    | |_____________^ method was supposed to return data with lifetime `'h` but it is returning data with lifetime `'1`

error[E0515]: cannot return reference to temporary value
   --> src/tx.rs:181:13
    |
181 |                &ReadTransaction {
    |   _____________^-
    |  |_____________|
    | ||
182 | ||                 tx: ReadOnlyRusqliteTransaction { conn: &self.tx },
183 | ||             }
    | ||             ^
    | ||_____________|
    |  |_____________returns a reference to data owned by the current function
    |                temporary value created here

I've tried casting as std::path::Path::new does for OsStr, but it says casting from rusqlite::Transaction to ReadOnlyRusqliteTransaction is invalid (but happily accepts the consequent cast from ReadOnlyRusqliteTransaction to ReadTransaction.

I don't quite follow how to get this to work. I understand in safe code I'm returning a reference to a temporary, but because both embedded types contain only a single reference, it should be possible to cast from &rusqlite::Transaction all the way up to a &ReadTransaction. Additionally the reference can't last longer than 'h, the lifetime of the rusqlite::Transaction's reference to the rusqlite::Connection. I assume I need to ensure that the reference to the write Transaction (&self) in deref, also doesn't outlive the reference to the rusqlite::Connection.

You can see my work around to provide a read method on Transaction just below, but I want to get the Deref to work for my understanding, and for a cleaner API.

2

Deref can only return data that has already existed before deref() has been called.

return &newly_created_object is invalid. If you force it with unsafe, it may cause use-after-free, and corrupt memory and crash the program.

This is because Rust's references aren't pointers for returning "by reference". References are temporary scope-limited loans of pre-existing data, and rely on having this data already stored somewhere for the entire duration of the loan. & is explicitly for not storing the data. It can't make anything live longer, and it can't prevent anything from getting destroyed.

When you create a new instance of a struct, it has to be stored somewhere. When it's stored in a local variable (let x = ReadTransaction {}), then it's bound to the scope of that variable, and will be destroyed when the variable goes out of scope (unless the data is moved to a different longer-lived storage, but Rust won't do that for you, and inside non-mut methods that's not easy).

Expression like return &ReadTransaction{} gets translated to let tmp = ReadTransaction{}; return &tmp, so you're still working with an instance stored in a local temporary variable, which will destroy its contents before the return returns. And you can't return reference to data that is getting destroyed.

To be able to return ReadTransaction, you would have to have a ready-to-use instance of ReadTransaction in self, so that you can return &self.existing_read_transaction. In that case return & works, because it's borrowing from self, and self is stored on some outer scope, outside of fn deref, so self and its data won't be destroyed when fn deref returns.

However, because ReadTransaction contains a reference to self.tx itself, you can't store it in self. This would make a circular reference (self-referential struct), and unfortunately that's another thing that Rust's temporary loans are not allowed to do. You'd have to use Arc<rusqlite::Transaction> and make clones instead of referencing bare rusqlite::Transaction.

There is one unsafe hack that you can do.

  1. Store rusqlite::Transaction as Box<rusqlite::Transaction>, so that your Transaction actually contains a pointer, which is stored in self and won't be just a temporary loan.

  2. Use #[repr(transparent)] on your ReadTransaction<'a>, which can only have one field, and this field must be &'a rusqlite::Transaction.

  3. return mem::transmute::<&rusqlite::Transaction, ReadTransaction>(&self.tx).

This can work, because Box<T> has the same binary representation as &T, and transparent struct wrapping &T also guarantees to have the same binary representation, so you can cast Box<T> to a struct with &T inside, and pretend that you've been storing that struct all along.

4 Likes
3

It's specifically this hack I'm trying to implement. I already have repr(transparent) on ReadOnlyRusqliteTransaction, and ReadTransaction. I hoped someone would actually try it out, since it's not as straightfoward as your explanation.

4

It would seem that transmute was the thing I was missing. I got it to work with

std::mem::transmute::<&rusqlite::Transaction, &ReadTransaction<rusqlite::Transaction>>(
    &self.tx,
)

I was trying something similar with casting to pointers and back to references with as. Is this style preferred to transmute?

Does the above code handle lifetimes correctly?

5

Isn't this returning a reference to a local variable?:

&ReadTransaction<rusqlite::Transaction>>(&self.tx)

The original error you got was:

error[E0515]: cannot return reference to temporary value

@kornel 's solution requires Box'ing the transaction as he mentioned.

2 Likes
6

If you make it &Struct transmuted from &cx, then you need to define Struct as owning a cx instance, otherwise it will mix up levels of indirection.

Having an instance of such struct anywhere would violate unique ownership, but I suppose if you only create it as a reference then it might be ok…?

7

I don't understand how Boxing helps here. I can obtain a reference to the rusqlite::Transaction from inside struct Transaction anyway?

8

Yeah in this case Transaction contains the variant of ReadOnlyRusqliteTransaction that owns its contents, so I believe I've done what you said. Can you illuminate why casting pointers would not work (unsafe { &*(x as *const y as *const z) })?

9

I didn't understand that you're returning two nested transparent structs containing a ref to Self, meaning you're expecting to just return a ref to Self. Whether this works correctly with transmute or your casting expression, I don't know, sorry.

10

The expression as you've written it will work, because the &* pair is special, merged into a single operation, rather than being a dereference followed by taking a reference to a temporary value. When dereferencing raw pointers it has ability to assign an arbitrary lifetime to the reference.

However, other cases of &expression that are semantically &{expression} will give you a temporary lifetime. unsafe does not extend any lifetimes, and does not change meaning of safe Rust code. So &Struct { val } remains invalid in unsafe blocks just as it is invalid outside of them (except when the struct is const literal, then it has 'static lifetime)

unsafe fn deref(ptr: *const i32) -> &'static i32 {
  return &*ptr;  // ok

  return &{ *ptr }; // not ok

  let tmp = *ptr;
  return &tmp; // not ok
}
3 Likes
11

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.