Implement `Into<&'a [u8]>` for `BorrowedBytes<'a>`

[sxyazi]

Currently, to get the underlying &[u8] represented by BorrowedBytes you need to go through Deref, Borrow<[u8]> or AsRef<[u8]>, but they all take a &self rather than self, which causes the returned &[u8] to be tied to a temporary &BorrowedBytes instead of to the lifetime 'a of BorrowedBytes<'a>, which is a reference to &'a mlua::String.

This makes:

fn as_bytes<'a>(s: &'a mlua::String) -> &'a [u8] {
  s.as_bytes().as_ref()
}

fail to compile:

cannot return value referencing temporary value
returns a value referencing data owned by the current function rustc (E0515)

By implementing Into<&'a [u8]>:

impl<'a> From<BorrowedBytes<'a>> for &'a [u8] {
  fn from(value: BorrowedBytes<'a>) -> Self { value.buf }
}

I can convert BorrowedBytes<'a> into &'a [u8]:

fn as_bytes<'a>(s: &'a mlua::String) -> &'a [u8] {
  s.as_bytes().into()
}

[khvzak]

Unfortunately it would be unsound.
Imagine the situation:

let lua = Lua::new();

let s = lua.create_string("hello, world")?;
let b: &[u8] = s.as_bytes().into();
assert_eq!(b, b"hello, world");

drop(lua);
assert_eq!(b, b"hello, world"); // use after free

[sxyazi]

Oh right, I forgot that we can drop the entire Lua instance, and BorrowedBytes needs to keep up to hold a strong reference to Lua, but its lifetime is tied only to the mlua::String

I noticed that when I call s.as_bytes() after drop(lua):

let lua = Lua::new();
let s = lua.create_string("hello, world")?;

drop(lua);
assert_eq!(s.as_bytes(), b"hello, world");

the current behavior is to panic with:

panicked at src/string.rs:119:30:
Lua instance is destroyed

In my scenario the Lua instance is not dropped early, so it can be guaranteed to keep available till the app exits.

Do you think it would make sense to introduce the same behavior (panicking when the Lua instance is dropped) for BorrowedBytes::into() as well? Something like this:

impl<'a> From<BorrowedBytes<'a>> for &'a [u8] {
  fn from(value: BorrowedBytes<'a>) -> Self {
    _ = value._lua.upgrade();  // Ensure the Lua instance is available
    value.buf
  }
}

and turn the existing _lua from a strong Lua to a weak one (Edit: on second thought, this would affect the existing AsRef<[u8]>, maybe it would be more appropriate as a method on mlua::String that returns a &[u8] instead of a BorrowedBytes - is there any reason we need a BorrowedBytes wrapper?):

pub struct BorrowedBytes<'a> {
  // `buf` points to a readonly memory managed by Lua
  pub(crate) buf: &'a [u8],
  pub(crate) borrow: Cow<'a, String>,
-  pub(crate) _lua: Lua,
+  pub(crate) _lua: WeakLua,
}

It might need a better name though since the semantics of into usually don't imply a panic.

[khvzak]

maybe it would be more appropriate as a method on mlua::String that returns a &[u8] instead of a BorrowedBytes - is there any reason we need a BorrowedBytes wrapper?

BorrowedBytes was introduced for transition from weak Lua inside String to a strong Lua inside BorrowedBytes.
You can actually create BorrowedBytes<'static> directly via FromLua conversion and then get underlying &'a [u8] bound to &'a BorrowedBytes<'_>:

let s = lua.create_string("hello")?;
let s2 = lua.convert::<mlua::BorrowedBytes>(&s)?;
drop(s);
let b = s2.as_ref(); // &'s2 [u8]

You can use BorrowedBytes<'static> as a direct replacement of LuaString
Strong Lua reference must be available for the entire lifetime of &[u8] otherwise memory can be freed while keeping reference to it.

[sxyazi]

Using mlua::BorrowedBytes<'static> as mlua::String works for my use case, but converting a &'a mlua::String to a &'a [u8] still feels kinda awkward to work with.

May I ask why doesn't mlua::String::as_bytes() just return a &'_ [u8] like mlua::BorrowedBytes::as_ref() does, instead of a mlua::BorrowedBytes<'_>, i.e. why is that intermediate mlua::BorrowedBytes<'_> needed?

I have a feeling that always using mlua::BorrowedBytes (with some added methods like to_str() which returns Result<str>) as mlua::String seems more idiomatic, i.e. I'd rather write something like this:

lua.create_function(|lua, b: mlua::BorrowedBytes| {
  let std_str: &str = b.to_str()?;
})

instead of

lua.create_function(|lua, s: mlua::String| {
  let std_str: &str = s.to_str()?.as_ref();
})

[khvzak]

May I ask why doesn't mlua::String::as_bytes() just return a &'_ [u8] like mlua::BorrowedBytes::as_ref() does, instead of a mlua::BorrowedBytes<'_>, i.e. why is that intermediate mlua::BorrowedBytes<'_> needed?

&[u8] reference, to be sound, need to be bound to two lifetimes 'lua: Lua and 's: mlua::String. The 'lua lifetime ensures that Lua VM is alive and 's that the string is allocated and not collected by GC.

mlua::String only has 's reference. It does not hold any references to Lua VM. The VM can be dropped at any time, invalidating string buffer.

mlua::BorrowedBytes is a common denominator, it keeps both references to 'lua and 's, to guarantee that underlying bytes are always valid.

PS Now I can remove lifetime from BorrowedBytes and BorrowedStr, as reference to String is cheap to clone (no longer require allocating a new slot on Lua reference thread).

[sxyazi]

Thanks for the detailed answer! Could mlua::String also hold a 'lua: Lua reference like mlua::BorrowedBytes does, or was there some design reason not to?

PS Now I can remove lifetime from BorrowedBytes and BorrowedStr, as reference to String is cheap to clone (no longer require allocating a new slot on Lua reference thread).

Sounds great, having fewer lifetimes is always a good call!