Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shouldn't decode return an &mut T rather than a &T? #19

Open
HadrienG2 opened this issue Sep 6, 2019 · 6 comments · May be fixed by #25
Open

Shouldn't decode return an &mut T rather than a &T? #19

HadrienG2 opened this issue Sep 6, 2019 · 6 comments · May be fixed by #25

Comments

@HadrienG2
Copy link

As far as I can see, there is no reason why users shouldn't be allowed to modify the deserialized object in place if they so desire.

@HadrienG2 HadrienG2 changed the title Shouldn't decode return an &mut T rather than an &T? Shouldn't decode return an &mut T rather than a &T? Sep 6, 2019
@frankmcsherry
Copy link
Member

For some types this is ok, but for other types like Vec<T> the ability to invoke &mut method could prompt e.g. de-allocation of memory that the allocator is not expecting (in this case, halfway through some bytes array). You would also gain access to mem::swap and other methods that replace the serialized data with owned data, and would complicate the state of the world once the &mut T is dropped (a bunch of owned data behind that should be cleaned up).

Does that make sense / were you thinking of a different reason?

@HadrienG2
Copy link
Author

Oh, I didn't realize that abomonation replaces the memory allocations of RAII objects with inline storage in its buffer. But in hindsight, it makes total sense, given how the exhume API looks.

A first question that comes to my mind is how you manage to get the pointer alignment right, but I guess this is already the topic of #8.

Also, this means that one must be extremely careful about implementing Abomonation for types that use internal mutability. It would probably be a good idea to mention this in the Safety section of the Abomonation trait documentation.

@frankmcsherry
Copy link
Member

Yup. Some of these things have quieted a bit as it became (years back) clear that Rust didn't have a stable story around unsafe, and I didn't want to make specific claims until they sorted that out more. Mostly, I didn't want to give the false impression that watching out for a few footguns would mean safety ensues. Instead, the front-page warning of "do not use this on data you care about" is meant to cover that. Also, text like

Be warned that implementing Abomonable for types can be a giant disaster and is entirely discouraged.

though it looks like I have the wrong name there.

I'll add some text to the trait documentation after "do not call these methods" and before "maybe don't use the trait at all" warnings. :)

@frankmcsherry
Copy link
Member

Btw, are you using this for something, or considering it? The crate exists mostly for a small-ish set of performance sensitive people working with big data, and it has other issues that limit broad applicability (e.g. no HashMap implementation, some other key owned library types). It's something I would love to make a bit more serious, but am blocked on the Rust unsafe team for a while, I think.

@HadrienG2
Copy link
Author

HadrienG2 commented Sep 6, 2019

I'm currently designing a log backend for real-time applications, and investigating the use of abomonation as a maximally-lightweight serialization mechanism for transferring log records from the real-time threads to the non-real-time logger thread. It seems that it could fit this sort of need better than a general-purpose mechanism like serde, and as a bonus it also gives me the "how large will serialized data be" info that I need for my evil lock-free programming tricks.

But obviously, before actually using it, I had to go beyond your "this library will eat your data and summon nasal demons" blanket statements and get a reasonably solid understanding of what abomonation's actual safety preconditions are. And this is how I ended up studying your API and spamming your bug tracker in the process ;)

By the way, you may be interested in the unsafe-code-guidelines repo if you don't know about it already. There were a lot of interesting discussions in there recently, and compiler people are available there to answer questions about how rustc currently behaves and whether they see a reasonable chance of it changing in the future.

@HadrienG2 HadrienG2 linked a pull request Sep 20, 2019 that will close this issue
@HadrienG2
Copy link
Author

HadrienG2 commented Sep 24, 2019

I think you should take a look at this RustConf talk by @oli-obk. The notion of ConstRefSafe that they define for the purpose of allowing heap allocations in const-eval seems suspiciously similar to the type contract that you need for replacing heap allocations with inline allocations in abomonation...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants