AioCb::from_boxed_slice violates mutability guarantees

[asomers]

An Rc<Box<[u8]>> does not have interior mutability. It's supposed to be impossible to update its contents. But AioCb::from_boxed_slice current allows you to. It does it by casting a *const c_void to a *mut c_void. Technically, that's a safe operation. The only unsafe part is when you dereference the pointer. But we have to do that in an unsafe block anyway, so the compiler never alerted us to the mutability problem.

Fixing this issue is trivial. The problem is that nix's consumers may be relying on this behavior.

[Susurrus]

@asomers Any progress with this? Been over a month and if this is trivial, I know you wanted the 0.10 release was supposed to happen quickly. It's only blocking on this.

[asomers]

The problem is a good solution isn't trivial. It's a question of what we want a good AioCb interface to be. Adding a new type of Keeper that safely allows mutable access is easy, but would pull in several dependent crates. I'm hesitant to do that. But if we don't do that, then we may as well remove the Rc<Box<[u8]>> type from Keeper, because it isn't very useful if it's immutable. That would force many Nix consumers to use an unsafe method to create an AioCb with a mutable buffer. So be it. But if that's going to be the primary API, then why bother keeping the Keeper structure around at all? The PhantomData type isn't very useful in many real applications. Some, but not many.

I've been grappling with this question, but there's no perfect answer. Should we just remove Keeper altogether, require unsafe methods for creating AioCbs, then create a new crate that safely wraps them for both mutable and immutable buffers?

[Susurrus]

Why is there a problem with pulling in new dependencies? If that allows us to offer a nice and safe API, what are the technical reasons not to do so?

[asomers]

Bloat. Bloat is the only reason. BTW, the three required crates would be bytes, iovec, and byteorder.

[Susurrus]

Bloat isn't a technical reason tho. What defines bloat and why is it bad? It's hard to make a technical decision when some of the information you're judging to make that decision isn't technical. So until I hear technical reasons, I'm inclined to say that's the best solution as I see no technical downsides to its implementation.

[Susurrus]

Fixed in #820.

It looks like GitHub doesn't auto-close issues when referenced from commits, only PR bodies. Lame.