facebook / sapling

A Scalable, User-Friendly Source Control System.
https://sapling-scm.com
GNU General Public License v2.0
6.18k stars 285 forks source link

up do date minibytes crate #897

Closed somethingelseentirely closed 1 month ago

somethingelseentirely commented 6 months ago

The minibytes sub-library is way too elegant and useful to be hidden away. It would be great if it was published regularly so that a recent version can just be pulled from crates.io.

somethingelseentirely commented 2 months ago

Just as an upgrade, I've decided to create a fork that tries to combine the best aspect of minibytes and ownedbytes called anybytes.

I've just discovered some UB in my library that I think also affects minibytes. When a Bytes is created from a Vec<u8> as the ByteOwner, then the reference returned fromdowncast_mut might be reallocated when the Vector is resized. Since the pointer in the Bytes themselves is not invalidated it can result in a use after free situation.

I don't really need a recent minibytes release anymore, so feel free to close this issue.

Thank you so much for your work and take care!

quark-zju commented 1 month ago

Sorry for the late reply and thanks for spotting the UB! It seems we don't use downcast_mut outside minibytes itself. So perhaps we can just mark it as a private function.

somethingelseentirely commented 1 month ago

I think that would be an option, although the act of handing out a & and &mut at the same time alone is probably UB. I changed the API to return an Arc<T>, where T is the type downcast to. That way you can only get mutability if all Bytes with that owner have been dropped.

quark-zju commented 1 month ago

handing out a & and &mut at the same time

Could you give an example about when that can happen? Given that:

It's unclear to me how we have & &mut borrows both alive at the same time.

somethingelseentirely commented 1 month ago

Yeah I think you're right, sorry my bad!

quark-zju commented 1 week ago

FYI minibytes is published now at https://crates.io/crates/sapling-minibytes

somethingelseentirely commented 1 week ago

Cool! I'll implement the appropriate traits in anybytes to make them compatible 😄