Added ability to let destructors get called

[schets]

This allows a program to specify that unlinked memory should be dropped when it's freed. This capability is needed for owning datastructures, like hashmaps.

One thing I'm not 100% sure on is how to handle zero-sized structs. From what I know, types that implement drop can't be zero sized due to the drop flag so this doesn't explicitly handle that case (and the test case uses a 'zero sized' dropping struct but I'm still not 100% sure.

Also, this implementation doesn't provide any ordering properties of destructors. It would be nice to have the property that all unlink_drops performed by the same thread will actually drop in the same order, but this doesn't hold when migrating garbage to the global bags. I think that using 4 global bags may fix this, I'm not sure how useful of a property this really is and whether affecting the rest of crossbeam is worth it.

[schets]

This should never be false for structs that implement drop, so there doesn't need to be an extra check and this will behave in a sane fashion for types that implement drop if my understanding of drop flags is correct.

[aturon]

Hey, sorry for the long delay on reviewing this -- in part because I'd been pursuing a separate branch that included some dtor support, which didn't ultimately pan out.

One basic problem here is that dtors are only safe for crossbeam to run if the data contained is Send and the dtor does not touch any non-'static data. The former, because the dtor might run on another thread. The latter, because the dtor might run after an arbitrary delay.

Of course, in many cases we want to use non-'static data, but that data is "removed" before the dtor would ever run (in the case of something like queue nodes).

Ideally, the library would give you some static protection here, perhaps through an unsafe StaticDrop trait that is implemented for all 'static data, in a way that can also accommodate things like queue nodes. Due to coherence restrictions, I haven't found a way to make this ergonomic yet.

Finally, I'd prefer not to have a flag -- I'd rather just always run dtors when the data is being torn down, and have you use a separate mechanism for cases like the queue where you want to pull out an owned message beforehand.

[schets]

Sorry for the late response - I've been out of town lately

dtors are only safe for crossbeam to run if [...] and the dtor does not touch any non-'static data. [...]. The latter, because the dtor might run after an arbitrary delay.

Is this true? I was of the impression that Rust didn't guarantee that destructors will ever run. Also, from the perspective of an individual thread in the system, this behaves fairly similarly to an Arc - the thread unlinks an object (drops the arc), and at some arbitrary, unspecified, possibly never point in time the object is collected (last arc is dropped).
The big differences that I see are that

• From the system perspective, the object remains alive after nobody holds a reference (I'm counting the GC as 'nobody', since from a user perspective, those references are useless).
• Crossbeam owns the data that it may destroy (how could it ever safely relinquish something?) and visiting threads only view the data.

Also, I don't think this system is all that useful if only static data can be owned by Crossbeam - wouldn't that mean all data owned by Crossbeam (and not just being shuffled around) has to exist for the entire program duration? Maybe I'm misunderstanding Rust's idea of static.

[sorear]

@schets Easier to answer the other way around:

Maybe I'm misunderstanding Rust's idea of static.

Yes, you are. A type satisfies the 'static lifetime bound iff it contains no non-owning pointers to ephemeral data. String: 'static is true, because a String owns the memory it refers to; &'static str: 'static is true, because the bytes it points to will be around forever; i32: 'static is vacuously true, as it contains no pointers of any kind; &'a: 'static is not true in general, because you are borrowing something that might not be around forever. But if regionck(?) sees that constraint, it will unify 'a = 'static, making the constraint true.

I was of the impression that Rust didn't guarantee that destructors will ever run.

Given:

let a = MyType1::new();
let b = MyType2::new(&a);

Rust guarantees that one of the following is true:

1. b.drop() is never called.
2. b.drop() is called before a.drop().

This means that b's destructor can safely use a, because if b is ever destroyed, a is guaranteed to still be valid. On the other hand, b can be mem::forgotten at any point before or after the destruction if a.

If b is stored in crossbeam, and crossbeam arbitrarily delays the destructor call, crossbeam needs to somehow ensure that b will not be destroyed after a.

[schets]

Ahhh, I was thinking of something like C++ static.

the dtor does not touch any non-'static data.

iff it contains no non-owning pointers to ephemeral data

Are these possible to satisfy in the Rust type system?

Having this isn't needed to write early-stage owning datastructures, but it's needed for them to be useful.

[sorear]

@schets

I was thinking of something like C++ static.

Well, you're not wrong, but at another level of pointyness. What T: 'static means is that if objects of type T are or contain non-owning pointers, then the objects they point at have lifetime 'static, in other words are global variables. This is a very slight hack, in that when people say T: 'static they usually conceptualize that objects of type T are not pointers at all thus satisfying the bound vacuously. T: 'static, and lifetime bounds more generally, does not constrain the lifetime of objects of type T, but rather the lifetimes of objects that objects of type T can point to.

Are these possible to satisfy in the Rust type system?

I'm sorry, I don't quite follow here.

@aturon

Of course, in many cases we want to use non-'static data,

I suspect we may be able to get away without supporting nontrivial lifetimes for a while. Didn't Send itself require 'static for a long time?

[Vtec234]

Ok, so I asked people on IRC about this. There is no way to assert that a type has a trivial destructor (one that does literally nothing) statically. There is an unsafe, unstable method for this, std::intrinsics::needs_drop. Your trait requirement T: Drop for unlinked_drop<T> is too restrictive - a type can have members that need nontrivial destructors, but not itself be Drop. It will always be needs_drop though.

I suspect we may be able to get away without supporting nontrivial lifetimes for a while. Didn't Send itself require 'static for a long time?

This could be the way to go. I can't think of a way for unlinked_drop<T: 'static + Send> to generate unsoundess when the epoch requirements are satisfied, but there may be some wierd cases.

Of course, given that unlinking is unsafe, we can just document the need for the programmer to assert all necessary preconditions for deferred destruction to be sound besides the usual epoch ones when calling unlinked_drop without encoding them in the type system. AFAIK there are no intrinsics to verify anything more specific about the drop behaviour, so this might be the only thing we can do if we want to support non-'static data.

[Vtec234]

@schets:
Crossbeam has just got new maintainership, so hopefully we can get this merged soon. If you could just change the bounds on unlinked_drop from Drop to 'static + Send and rebase on current master, this should be good as an initial implementation.

[schets]

@stjepang

[stjepang]

Bounds on T should be a bit stricter, but otherwise this is good to go.

[stjepang]

Bounds on T should be a bit stricter, but otherwise this is good to go.

[stjepang]

Like @Vtec234 said, this function needs T: Send + 'static.

• Send because some other thread might pick up the job of destructing val.
• 'static because we don't know how soon val will be destructed (potentially never).

EDIT: Removed T: Drop bound (@Vtec234 explained why it's too restrictive).

[Vtec234]

No, it should not be Drop as I already explained. Only Send + 'static.

[schets]

Oops, I wasn't really thinking when I added that

[sstewartgallus]

Instead of having flag arguments like this you could have separate functions. For example, insert and insert_and_drop. What do you think about that?

[arthurprs]

🎉

[arthurprs]

🎉

[Vtec234]

I just noticed that there is no documentation on unlinked_drop and unlinked doesn't mention not running destructors. Here's how it could look like:

/// Assert that the value is no longer reachable from a lock-free data
/// structure and should be collected when sufficient epochs have passed.
///
/// Bear in mind that `unlinked` does not register data for deferred destruction.
/// The occupied memory will be freed as if it were a primitive array of bytes.
fn unlinked

/// Assert that the value is no longer reachable from a lock-free data
/// structure and should be destroyed when sufficient epochs have passed.
///
/// Unlike with `unlinked`, data registered for recollection using `unlinked_drop`
/// will have its destructors run. This, however, also puts additional restrictions
/// on the kinds and lifetimes of data that can be passed to this function.
fn unlinked_drop

[schets]

I'm going to put this on hold in light of #134, since various aspects of that proposal could change how destructors are managed