(Just small thing I've been thinking about recently. No concrete proposals or action items, but I figure sharing the problem space would help).
In Rust, fields are dropped in the order of declaration, which is done mostly for the sake of determinism. This gives you full control over drop order, but does not allow one to distinguish between (super majority) of cases where any order would work, from cases where a specific order is required.
When drop order is significant, it's important to leave visible marks in the source code about this. The current suggestion is to use ManallyDrop and a manual Drop impl, but that:
is unsafe
requires to pay "wrapper struct tax" everywhere
requires to write an explicit Drop impl, which affects dropck (example).
Note that although often, if you worry about drop order, you have unsafe some else, this is not always the case. The case I hit constantly is this:
struct Worker {
sender: Sender<Item>,
// Handle to a thread which joins on drop and which runs
// for item in receiver { }
// loop
thread: jthread::Handle,
}
One can rely on the comment, but that feels meh. Moreover, the comment pins down total order, which is obscures order constraints that are actually interesting.
So, it might be interesting to have some mechanism to signal drop-order in the source code, with the strawman syntax being:
On unsafe: one thing I'm fond of about the invariants for ManuallyDrop is that the unsafe can be restricted to just the Drop::drop method, where it's one of the easiest kinds of unsafe to review.
So that plus the fact that struct order is now guaranteed means that I'm not sure how worth it special syntax here would be. Doing it safely by order with a comment and a #[test] might just be fine.
Which is slightly less of a nit because at least ManuallyDrop will Deref -- safely -- to the underlying type, so it's substantially less annoying to use for this than MaybeUninit would be.
The provided example does not demonstrate the need to cause out-of-order drop behavior. As noted, a comment—and thus no language churn—is sufficient for the example.
IMO a #[drop_order = after(…)] annotation is only really needed when field alignment requirements coupled with required drop order lead to a non-dense struct that has so many occurrences that the wasted memory is significant. I have never observed such a case in all the code I've reviewed.
Note that for this case, using an Option (or another mem::replace-like take pattern) can replace the unsafe-ty:
Option
struct Worker {
/// Wrapped in an `Option` to be able to drop it earlier (on `Drop`)
sender: Option< Sender<Item> >,
thread: jthread::Handle,
}
impl Worker {
fn sender (self: &'_ mut Self) -> &'_ mut Sender<Item>
{
self.sender.as_mut().expect("Already dropped `sender`??")
}
}
impl Drop for Worker {
fn drop (self: &'_ mut Self)
{
// drop the `sender` before the rest / the `thread` handle.
drop(self.sender.take());
}
}
▼ mem::replace
struct Worker {
sender: Sender<Item>,
// ...
thread: jthread::Handle,
}
impl Drop for Worker {
fn drop (self: &'_ mut Self)
{
// drop the `sender` before the rest / the `thread` handle.
drop(::core::mem::replace(&mut self.sender, channel::new(...).0));
}
}
That is, I feel like the moment this is perceived as a "drop earlier" kind of issue, rather than a drop later, the take pattern becomes more intuitive / idiomatic.
Regarding the OP itself, I would say that a comment on either sender or thread reminding people to pay unusual attention to declaration order in that it suddenly is semantically meaningful is a quite decent solution
in that regard, a procedural macro to enforce field orders would solve this issue and be quite trivial to implement:
#[derive(FieldOrderSensitive)] // Hints at something non-trivial going on here
struct Worker {
sender: Sender<Item>,
#[assert_declared_after(.sender, reason = "needs to be dropped after it")]
thread: jthread::Handle,
}
@matklad I see what you're trying to accomplish, but can you please explain how this is better than what @dhmcame up with? Or how this beats the more general purpose Arc?
I can't explain why this is better, as my proposal is explicitely a strawman, but I can explain why the suggested solutions (which were considered) do not entirely satisfy me.
ManuallyDrop -- unsafe, minor inconvenience at call sites, needs explicit Drop impl (dropck implications)
Option -- poisons all call-sites with "can't happen" unwraps, Drop impl
replace -- not always possible, Drop impl
procmacro -- works perfectly, but has high cost
comment -- the best solution, and this is what I use right now, but:
over specifies the drop order
putting important invariants into comments is suboptimal, comments bitrot, are not processed by IDE refactors, and are easy to skip over.
OK, I see your points. Let me ask a weirder question then. Is there any way for this to cause a deadlock? That is, will the compiler always be able to detect something like the following at compile time?:
In the above code this isn't that big a deal for the compiler to detect, but what about trait objects, or anything else where the precise type is obscured and possibly unknown at compile time?
It seems so -- the ordering is local (defined on a single struct), and checking/implementing it involves finding any toplogical sorting of the fields with respect to drop_after relation.
Agreed. Analysis of the precedence relationships either lead to a realizable partial ordering or detect the existence of one or more circular ordering requirements.
A different angle: if Rust had EagerDrop ( == "I don't care when it's dropped"), then it could also be extended to mean "I don't care about order in which it is dropped".
Then the old Drop would become the kind of drop that is sensitive to these things, so implementing Drop instead of EagerDrop would become an indicator that it matters how the object is dropped.
@matklad, @Tom-Phinney, are you sure? I know I'm being picky here, but I also know that at some point someone is going to design a crate that accepts trait objects as callbacks, and for some reason they'll be required to drop in a particular order. Debugging that will be a nightmare. I'd like to prevent it from occurring if at all possible.
Just to be 100% clear, I'm not against this proposal; I really do see your points, and they make sense to me. I just don't want to introduce something that is potentially a very subtle footgun. If you can develop rules similar to the rules for mutexes in C that ensure deadlock is impossible, then I'd be fine with this.
@kornel I like the idea of EagerDrop, but I think it would be better to have something like OrderedDrop instead. That way the current drop semantics would be EagerDrop, and the new OrderedDrop (which is a restricted subset of Drop) would continue as it has. This would mean that old code that could be dropped in any order continues to work as it always has.
The drop ordering annotations would be on fields, not types, so it's entirely local information and not impacted by the type of the fields (and thus, if dynamic dispatch is involved; field drop order is still a compile time constant, not a constraint solved at runtime).
@CAD97 I understand what you're saying, but my concern is the more complex types that can be constructed. If you have something like:
struct Droppable;
impl Drop for Droppable {
fn drop(&mut self) {
println!("Dropping Droppable!")
}
}
struct Foo {
a: *const Droppable,
#[drop_order = after(a)]
b: *const Droppable
}
struct Bar {
c: Droppable,
#[drop_order = after(c)]
d: Droppable
}
//
fn main() {
let bar = Bar{c: Droppable, d: Droppable};
let foo = Foo{a: &bar.d, b: &bar.c};
}
What will happen here? If I understand things correctly (and I haven't made too much of a mess with my example! ) b cannot be dropped until a is dropped. a points to d, and b points to c. d must be dropped after c. For c to be dropped, b must be dropped (or you have a dangling pointer, right?). And b cannot be dropped until a is dropped.
Have I missed something? If you comment out the drop attributes, you can run this on the playground, and everything gets dropped correctly.
Dropping a pointer doesn't do anything, so there's no observable effect of drop order on those fields.
I think you are making this more complicated than it actually is. This proposal only adds alternate syntax for something you can already do by changing the order of fields in the struct declaration.
Yes: Regardless of the ordering within each struct, foo is droped before bar, so there is no possibility of a dangling pointer. Both a and b will always be dropped before c and d are dropped. Changing the drop order within each struct has no effect on that.
This might have been written when the text still said MaybeUninit -- with ManuallyDrop, as was mentioned above, you only play wrapper struct tax in the constructor(s).
And unsafety is also limited to calling drop in impl Drop. So I'd say this is a minor point and I am not convinced it is worth a new language feature. FWIW, controlling drop ordering is explicitly listed as a usecase of ManuallyDrop.
OTOH, the dropck effect is a much more severe problem. But instead of designing a system for drop ordering to avoid manual Drop impl's, I'd rather work on getting dropck into a shape where it can be stabilized.
)