search

kyren / gc-arena

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

Add weak pointers `GcWeak` and `GcWeakCell` #16

Closed Aaron1011 closed 1 year ago

Aaron1011 commented 1 year ago

This is a combination of work by EmperorBale and me. It adds weak-pointer version of Gc and GcCell, named GcWeak and GcWeakCell respectively. These can be created by calling downgrade() on the corresponding 'strong' struct (Gc/GcCell).

You can attempt to upgrade a weak pointer to a strong pointer by calling upgrade. This will succeed as long as the target object has not yet been freed (it will also fail during Phase::Sweep when the target object is going to be freed, but has not yet been freed).

To support this, GcBox now has two additional flags: alive and has_weak_ref. When an allocation with weak pointers is garbage-collected, we just run the destructor on the Collect value if there are still weak pointers. The GcBox itself is not deallocated, so that the weak pointers are still able to access the flags. In a later Phase::Sweep where all of the weak pointers are gone, we actually deallocate the memory for the GcBox.

I've also added comments to existing code, to make it clear how weak pointers interact with the existing invariants.

kyren commented 1 year ago

Sorry this took me a while to get to, I'm actually reviewing the PR now but it's a lot to load back in my head. From the first scan I did, the additional comments you wrote about how the gc works are just :ok_hand:

kyren commented 1 year ago

Okay, after reading over this and... hopefully... remembering how this garbage collector works and reviewing the change, I think the reasoning here is sound.

I have some concerns but they're mostly nits..

kyren commented 1 year ago

@EmperorBale and @Aaron1011 you both might understand the gc better than me at this point considering how long it's been since I've thought about this seriously, so if any of my feedback is just wrong, please point it out. Some of the documentation corrections I suggested were honestly to make sure I understood it myself, maybe i have it wrong? I don't remember half the things I do lol.

Aaron1011 commented 1 year ago

@kyren Thanks for the review - I've pushed a new commit addressing your comments.

kyren commented 1 year ago

Thank you for the PR! Also thank you for running under miri, that was gonna be the next thing I did! I'd like to have that as part of CI as well but I'm not asking you to add that in this PR.

kyren commented 1 year ago

LGTM!