jonhoo / left-right

A lock-free, read-optimized, concurrency primitive.
Apache License 2.0
1.95k stars 94 forks source link

Destroy the map when WriteHandle is dropped #25

Closed jonhoo closed 5 years ago

jonhoo commented 5 years ago

The old code was leaking pretty much the entire map in the (common) case where WriteHandle::destroy was not called. Here's why:

If the writer is dropped first, they first swap twice to make the two maps identical, and then forget all the records + drop the write-side of the map. When the last reader goes away, they drop their ReadHandle, which drops the last Arc<AtomicPtr<Inner>>, but that does not in turn drop the Inner! If the reader is dropped first, the effect is the same, since the dropping the last reader's Arc<AtomicPtr<Inner>> doesn't do anything.

I first figured I could solve this by the AtomicPtr<Inner> being replaced with a newtype wrapper that drops the Inner when it is itself dropped, but then if the last reader is dropped first, it in turn drops the Inner, which de-allocates the map (and all the contained values). When the writer is later dropped, it now has lots of dangling pointers (due to ShallowCopy), a null pointer that it doesn't expect from the reader side, and it doesn't know that it should be forgetting all of its values.

So, the solution here is to simply destroy both sides of the map when the writer is dropped, which we can do safely. It's not ideal, but at least it doesn't leak. Users who want the old behavior can either leak the WriteHandle, or just stick it in a global Mutex somewhere. It does sadly change the API though, so we'll need a major version bump.

Fixes #24.