No, I do not believe that API could possibly be made sound (and I don't think its a priority to make it sound regardless).
(Discussion on making unleakable types is off topic for a thread about async destructors, but I'd direct you to this post about our decision not to add this sort of auto trait in 2015).
While reading the post, this was mentioned a couple of times:
This helps avoid the problem of types that don’t properly implement
Dropwhen they implementAsyncDrop
I didn't see the option of making Drop a super-trait of AsyncDrop discussed, is there a fundamental reason for this? I'm not sure if the AsyncDrop trait proposed at the beginning adds any extra value over the final solution. It would allow T: AsyncDrop bounds, but I don't know if that would be useful (Drop trait bounds are weird).
Drop trait bounds are not only weird, they're not allowed. (As a reminder, users are also not allowed to call the drop method directly). The async destructor would have the same conditions, and so AsyncDrop: Drop would just seem to me like a less convenient way of adding an async drop method to the Drop trait.
My thinking here is that such an API would be ideal because it mirrors the sync API and has a zero-cost implementation and that making major changes like the one proposed here without a proven plan to support the ideal API risks going through the pain of the change without getting the ideal benefit.
As for the statement that it "could not possibly be made sound", doesn't making that future unforgettable and undroppable (except for async drop) make it sound? (obviously with some design drawbacks)
It also seems to me that this is on-topic also because types that need to be async dropped should not be sync-droppable and non-sync-droppability and unforgettability are similar concepts (not blocking in drop is not just an optimization, it is necessary for a properly functioning program that is not susceptible to either hanging indefinitely or creating an unbounded by a constant number of blocked OS threads and running out of RAM).
EDIT: Also something that is interesting to note is that only non-'static Futures would have to be non-sync-droppable and non-forgettable (because 'static Futures can be dropped by spawning async-drop on the executor), so the "split" between droppable/forgettable and non-droppable/forgettable types should essentially be limited to future combinators, since most other code would either not use a Future or use a 'static Future.
Remember: one a future has been polled once, it's been pinned. If it is !Unpin, it is unsound to forget a future once it's been started.
Due to this, I think (though don't have the proof) that a completion-based future that doesn't own the buffer can actually be made sound.
The basic API, super sketch:
struct CompletionFuture<'a> {
state: State,
buffer: &'a mut [u8],
pinned: PhantomPinned,
}
impl Drop for CompletionFuture {
fn drop(&mut self) {
let this = self;
pin_mut!(this);
block_on(Future::from_poll(this.drop_poll));
}
fn drop_poll(Pin<&mut self>) {
match self.state {
State::AwaitingCancellation => self.check_cancellation(),
State::Cancelled | State::Done => Ready(()),
State::New => Ready(()),
State::AwaitingOp => self.request_cancellation(),
}
}
}
impl Future for CompletionFuture {
fn poll(Pin<&mut self>) {
match self.state {
State::New => self.request_operation(),
State::AwaitingOp => self.check_operation(),
State::AwaitingCancellation => self.check_cancellation(),
State::Cancelled | State::Done => Ready(()),
}
}
}
Leaking the future after it's been started would require the loan of the space to last forever, IIUC. Also IIUC, just boxing the future then forgetting it is also unsound even though the future's location itself isn't reclaimed, because the value at that spot is invalid once the lifetime 'a expires.
This is definitely not true, and it sounds like its probably based on misapplying the word "invalidate" as its been defined by some of the UCG work to the way its being used in reference to pin.
You can mem::forget a Pin<Box<T>> even if T does not implement Unpin. Its completely safe and we can't assume it won't happen. All thats protected is the actual memory representation of T, which cannot be overwritten unless the destructor runs.
However, this is still off topic for this thread. Async destructors have applications outside of the completion/cancellation problem, and there are other threads on internals for discussing ways to solve the completion/cancellation problem.
On one hand I like the idea of continuing not to insert secret fields into users' types. On the other hand, these secret fields would be exactly equivalent to the current on-stack drop flags, and I like the idea of consistency with sync code.
This suggests a third approach: Depending on how frequently we expect people to use hand-written futures as trait objects (rather than directly from an async fn), we could just expect such types to implement their own drop flags for poll_drop when necessary.
This is more onerous than implementing a fused poll_drop_ready, but if that case is rare enough then we get to keep poll_drop and most of the time people won't need to write anything.
(Or, alternatively, a fourth approach: make "fused" a requirement of all poll_drop_readys, and generate them as drop glue.)
Not sure how this could work, I'm not sure you've understood the problem in the same way I have. Consider that Bar and Baz both have async dtors:
struct Foo {
a: Bar,
b: Baz,
}
let x: Box<dyn Any> = Box::new(my_foo);
We need to implement the drop glue for Foo, not for Baz and Bar. Baz and Bar already will have their own "drop flags" in essence as part of implementing poll_drop_ready, but if we want to guarantee we won't call poll_drop_ready after it returns Ready, we'd need additional state in structs like Foo.
Not sure how this is different from the second approach I listed.
Right, I'm suggesting that in this example we just wouldn't async-drop Foo. This is "okay" because async drop is purely an optimization.
If the author of Foo did want to be async-dropped when used like this, they would need to add extra state to Foo to track which of a and b they've dropped.
But presumably Foo would more often be used as a local to an async fn, where those drop flags could be part of that future.
It's the same, but automated by the compiler. If that's what you meant then it's identical. 
To be fair, you explicitly brought up the completion/cancellation problem in the blog post as a "really interesting low level use case" for async destructors.
But whether it's worth continued discussion in this thread really depends on whether, as @bill_myers suggests, a solution to that problem can be had which would also change the design of the asynchronous destructors feature – apparently by having the type system make it impossible to sync-drop an object that wants to be async-dropped. I'm fairly skeptical that that could actually work, though.
It seems to me that the time to fix the problem would have been before Pin was stabilized. Stack-allocated pins have the property that they cannot be forgotten, thus lifetimes in them cannot expire before the pinned object is dropped.* If we had somehow enforced the same requirement for Box::pin and other non-stack pin functions, perhaps by having them require T: 'static (at least until a more comprehensive solution could be devised), then "lifetimes cannot expire" could have been part of the Pin guarantee. However, it doesn't seem like anybody realized this guarantee would be useful until Pin was already stabilized.
Personally, I still think that although Pin was an elegant design within its constraints of not requiring compiler changes, it ought to eventually be deprecated in its entirety in favor of some kind of !Move-like solution once the compiler has had time to catch up. Unfortunately, it seems increasingly difficult to do such a thing backwards compatibly, which means it probably won't happen at all.** If such a transition did happen, though, it could theoretically provide an opportunity to revisit and extend the guarantee.
* Except for "stack" pins within async fns. They can currently be forgotten if the Future representing the async fn is itself forgotten, but if the latter were impossible, the former would be too.
** I wish we had limited Pin to specific pointer types like &T, &mut T, Box<T>, etc., rather than any P: Deref. If we had, it would be possible to eventually turn e.g. Pin<&mut T> into an alias for &mut Pinned<T>, for some Pinned type that would be !Move and thus not subject to mem::swap.
What happens when you drop something with an async destructor in a non-async context? There are two options:
- Call it’s non-async destructor, like every other type.
- Introduce some kind of executor to the runtime (probably just block on) to call as part of the drop glue.
Are those the only two options, or could we make this a compile error?
We could add an auto trait that allows a type to opt out of the default destructor: impl !Destruct for MyType {}. Then we say it’s a compile error for a value to go out of scope unless (1) its type implements Destruct, or (2) we’re in an async function and its type implements AsyncDrop, or (3) it’s destroyed explicitly by pattern-matching it in a module with access to its private fields.
There are other potential use cases for non-default-destructible types. We could have types with async destructors, multiple destructors (like a DB transaction to be destroyed with commit or rollback), destructors that take extra arguments, or destructors that might return errors via Result—these would all just be normal functions that consume their argument by value, and the compiler would enforce that we don’t forget to call one of them (except, as always, by leaking).
What would be the guarantees for poll_drop being called? My understanding is that if this is just added selectively to some Futures, any Future in the chain which isn't aware of poll_drop will just drop the Future - and the async destructor would not be called. So all Futures, async fns and combinators would need to get updated. If they are not, then the implementor of poll_drop can't really rely on it being ever called.
The only exception would be if drop also gets updated to call poll_drop. It could drive the executor inline until the subtask is finished - but those things have a super-high chance to lead to a deadlock (e.g. if another task also requires synchronous drop/completion). That's an experience that was already made in other environments like QT or Win32.
If we go towards the step that every type gets updated with a poll_drop() method then I actually don't see this being a lot different than
Future type, where poll is an unsafe function and must always be driven to completionuncancellable async fn(), which gets desugared to the new Future type, and which could call regular async fns but could not be called by them.Well they currently are allowed, but useless. (Except the fact that they are allowed throws a wrench into some plans for const fn 
)
They also can be used to specialize on Drop, allowing you to tell "types that have drop glue because they implement Drop" from "types that have drop glue and don't implement Drop" apart. I'm pretty sure that we can get creative with this... but why...
You can't rely on it getting called, but we do generate drop glue, just like we generate normal drop glue already. It's not the case that every type would need to implement poll_drop_ready to visit all of its fields, its much less severe than that:
Future::poll method need to poll_drop_ready them.But combinators that just hold their futures until the end and then drop them will, of course, have poll_drop_ready called when they're consumed by await, and so on. It's a very narrow set of cases that need to manually handle this, just as a narrow set of cases already need to manually call drop_in_place (like collection types).
This isn't how I remembered it working. Thanks
Currently Deref::deref, DerefMut::deref_mut, Drop::drop are three "operation"s with different behavior. Instead of replacing the operation itself, they're executed prior to the "actual operation", like a hook, so
f(*v) first call Deref::deref() on v or &v, getting an &Target, then execute the *<x> part on the &Target and finish the call on Target
*v = 5 first call DerefMut::deref_mut() on v or &mut v, getting an &mut Target, then execute the *<x> = 5 part on the &mut Target and finish the assignment.drop(v) first call Drop::drop on &mut v, then finish the destruction on <x> internal fields.When async destructor become a thing, i'd like to see some consistency there if possible...
I'd like more explanation of the problem/tradeoffs with additional state in the naive version
trait AsyncDrop {
async fn drop(&mut self);
}
Or the closest you can get without async trait fns:
trait AsyncDrop {
type F: Future<Output=()>;
fn drop(&mut self) -> Self::F;
}
The example that comes to mind is using Box<dyn X> in async code, where X is some object-safe trait, and we want async drop to work properly. How about just not implementing this? If someone wants a box that supports async drops, they can use a separate type with whatever memory layout they want. One way they could do this is to implement poll_drop on X and let MyAsyncBox<dyn X> use X::poll_drop, but this is flexible.
Benefits of the naive version are that futures can be easier to use, and it can avoid adding extra state fields to objects.
The problem with the naive version is that the code that runs the async destructor has to allocate space for type F: Future<Output=()>;. This isn't something easily known statically, and hence results in alloca type stack tricks, or Boxing of the resulting future to make it runnable by the glue code, even when that Box is simply overhead as the Future is stateless.
Having thought a bit, I like the poll_drop_ready proposal better than poll_drop - rephrasing the problem slightly, the current situation is that sync drop can block a thread waiting for operations to complete. We want some way to ensure that, when running in an async context, drop does not need to block the thread - none of its code calls an I/O operation.
My understanding of poll_drop_ready is that it enables a Future author to await any work that the drop function will have to block on; hence, poll_drop_ready loops leaving the Future in a valid state, and drop actually does the work of dropping the object.
This means that no-one ever needs to implement poll_drop_ready; you can always rely on drop being called, but cannot rely on poll_drop_ready; instead, the semantics of poll_drop_ready are that after poll_drop_ready has returned Ready, drop will not block as long as no other changes are made to the object's state.
Then, the drop glue is the moral equivalent of:
if in_async_scope {
poll_fn(obj.poll_drop_ready).await;
}
obj.drop();
// and obj's memory is freed here
I don't think there's a need for poll_drop_ready to be fused - by moving Futures into a new async task, you can get the effect of dropping the Futures on a background task if the runtime supports that. This does, however, imply that for optimality, the drop glue should join_all the poll_drop_ready work, so that all objects are prepared for drop concurrently at the end of a scope in a task, and then drop is serial again in currently defined order.
I agree pretty much exactly with your post @farnz, and that's the design I was planning to move forward with. There is only one comment that confused me:
The two parts of this sentence don't have a relationship that I can understand. Could you elaborate further on what you mean?
To be fused means that calling poll_drop_ready after its already returned Ready should continue to return Ready - and also that implementers should assume that this is possible to happen (whereas many futures simply panic if you call poll after you receive Ready). The reason for this is that structs' "poll_drop_ready" glue needs to be stateless since it has nowhere to store that state, and so it just calls poll_drop_ready on all fields until they all return Ready.
This isn't related to running async destructors on a second task or not, which I agree should be done by the user explicitly.
I'd missed the need for struct's poll_drop_ready to be stateless, which is where my confusion there comes from - I had thought that the need for it to be fused came from a desire to run poll_drop_ready in a spawned task so that when you exit the scope doing work, the task cleans up immediately.
My error, not yours.