kyren / gc-arena

Incremental garbage collection from safe Rust
Creative Commons Zero v1.0 Universal
436 stars 36 forks source link

Make `Arena` conditionally Send if the root type is Send #61

Open kyren opened 1 year ago

kyren commented 1 year ago

Also makes Gc<'static, T> Send, so that the Arena has a possibility of being send.

I am not totally sure on the soundness of this. It doesn't give any extra powers to any types other than the Arena type and Gc<'static, T>.

It shouldn't be possible to get into trouble with Gc at all, since you can't obtain a Gc<'static> in the first place (and even if you could safely, it could never be freed so that's fine it is totally not fine, and you totally could get a Gc<'static> which could make a !Sync type into a Sync one and I had to change the mutate signatures to prevent it).

You shouldn't be able to get in trouble with Arena, because it is always !Sync, and you won't be able to store anything !Send (other than Gc) inside of it.

A question I have is if there's some way to get in trouble with the projection of the root type, by applying the lifetime to a different type, to get a thing that mentions a type which is only Send if it is 'static (similar to Gc here), and have that be transmuted to 'static, but that can't be safe to do anyway since it would have to be an unsound lifetime transmute, since the 'gc is generative and can't be safely related to whatever type is being projected... I think?

Making Send Arenas would be heckin useful to me, because in ECS design, !Send types are generally always low level painful.

Anyway this is here to be discussed, feel free to crush my hopes and dreams.

kyren commented 1 year ago

This turned out to be a whole thing in the presence of dyn Trait types, which are somewhat unergonomic to make + Send. I'm sure it's possible to do via an unsized_send macro as discussed on Discord, but I'm too tired to figure it out right now.

I have cooled somewhat on the idea after seeing some of the implications of it, so I'm just going to leave this for now. I might revisit this idea if it becomes crucial to me, but right now it's more of a pain than pinning to a thread.

kyren commented 1 year ago

This turned out to be a whole thing in the presence of dyn Trait types, which are somewhat unergonomic to make + Send. I'm sure it's possible to do via an unsized_send macro as discussed on Discord, but I'm too tired to figure it out right now.

I was no longer too tired to figure it out, it's not that bad at all, PR is revived as it was, hype back to max.

kyren commented 1 year ago

After mulling it over, I'm much more confident that this is sound.

Treating 'gc as 'static must be sound for the same reasoning that underpins the existence of the entire crate.

The only way to access the inside of an arena is to provide a callback that must work for all lifetimes 'gc. Since that callback works for all lifetimes, it must work for the 'static lifetime. The safety of the garbage collector in general relies not on the 'gc lifetime being some unique lifetime, but rather that the callback cannot know what lifetime it is.

The only things directly produced that have the 'gc lifetime are inside the callback and are from gc-arena provided functions. gc-arena can do this, and allow them to be stored in the root object, because gc-arena knows that the 'gc is really 'static.

The safety of the garbage collector derives from the lack of information of the callback, not any specific lifetime. It cannot know what the 'gc lifetime really is, so it must assume it could be non-'static, as well as not allowing it to be unified with any other lifetime (since the provided 'gc is also forced to be invariant). This is what prevents Gc pointers from escaping the arena, being put into thread local storage, being put in a mismatched arena etc, and also with this PR what prevents code from assuming that they are Send.

Being able to project the 'gc lifetime to 'static within unsafe gc-arena code must not cause unsoundness in and of itself, because it is really always 'static, the callback is just not privy to this information. As long as no user code in the callback or outside the arena can witness a 'gc lifetime being 'static, it cannot use this truth to do something unsound, like send Gc pointers to another thread or store them anywhere.

So, I think this PR is sound, at least in that way.. treating 'gc as 'static within gc-arena code is fine, and is actually what all of gc-arena already does. As long as no safe code can ever observe that 'gc is really 'static, we are okay, but that is the foundation of the entire crate.

I feel like I'm repeating myself, but anyway I'm more confident about all this now, and this was the one thing that I was really unsure about. Anyway, I hope all this makes sense, but if it doesn't please tell me!

kyren commented 1 year ago

WELL, being sound doesn't make it easy to use...

The extra macro mode that I had worked in very simple cases but immediately fell over in complex ones, I'm still not 100% sure of all the reasons, but it has to do with the fact that you need to implement traits (which may be parameterized over 'gc) for the 'static projection, not the 'gc one, because that's when the pointer cast happens. This is basically unavoidable, because you need the cast to happen with something that points to a Send type and something that implements the trait you want to cast to, and the ONLY way that can happen is if the trait is implemented when projecting to 'static. It is frustrating that we can't somehow prove to rust that the original lifetime is 'static using unsafe, but I couldn't figure out a way.

Instead, I'm going to try an alternate approach which is not really any worse: an EnsureSend wrapper type!

It's definitely easier to create than using the macro, and it's a slightly worse type signature where it is declared.

kyren commented 1 year ago

I think EnsureSend is not sound, even though I'm having trouble coming up with a counter example.

Other than EnsureSend or the unsize macro, the logic seems solid, but everything around dyn casts is becoming awful.

I think EnsureSend would totally be sound IF EnsureSend had a Rootable type parameter. This is not that unreasonable of an idea, and I tried it, but then the problem loops entirely around and we can't do proper unsize casting anymore. You might be able to get the unsize casting back with a macro for dyn casting between projections, but... I think we're back to a custom GcSend pointer type.

I can't stand to think about this anymore and I think I'm going to drop this idea for the moment, or at least pause on it and let it stew for a while.

kyren commented 1 year ago

Yet again, I failed to drop it.

I am back on the side of thinking EnsureSend is sound, but not for the reasons I gave (instead, for simpler reasons). I have changed the struct name and explanatory comment to reflect my new, simpler logic.

kyren commented 1 year ago

I thought about it a lot more and decided that making !Send arenas safe through thread pinning is actually less work than maintaining this API, and I don't really lose much by doing so.

Having Send-able arenas is still a nice ergonomic thing though, but now that I understand that both sides are mostly just questions of ergonomics, it's something I can actually evaluate.

For now, I don't think this is worth it, but I'm not gonna close the PR or anything, just leave it in its current working state until such a time that somebody feels it is worth finishing.