Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

entries are leaked when sync::Cache is dropped #383

Closed
igorzi opened this issue Jan 19, 2024 · 5 comments · Fixed by #384
Closed

entries are leaked when sync::Cache is dropped #383

igorzi opened this issue Jan 19, 2024 · 5 comments · Fixed by #384
Assignees
Labels
bug Something isn't working
Milestone

Comments

@igorzi
Copy link

igorzi commented Jan 19, 2024

Simple repro below (Foo is not dropped when the cache is dropped).

use moka::sync::Cache;

struct Foo {}

impl Drop for Foo {
  fn drop(&mut self) {
    eprintln!("Foo dropped");
  }
}

fn main() {
  let foo = Arc::new(Foo {});
  let cache = Cache::builder().build();
  {
    cache.get_with("foo", move || {
      foo
    });
  }
  drop(cache);
}
@igorzi
Copy link
Author

igorzi commented Jan 19, 2024

Note that if I add cache.run_pending_tasks(); right before drop(cache); - the entries do get dropped.

@tatsuya6502 tatsuya6502 self-assigned this Jan 20, 2024
@tatsuya6502 tatsuya6502 added the bug Something isn't working label Jan 20, 2024
@tatsuya6502 tatsuya6502 added this to the v0.12.4 milestone Jan 20, 2024
@tatsuya6502
Copy link
Member

Thanks for reporting!

I believe this is due to the spec that crossbeam-epoch crate does not guarantee to run the destructor of the pointed object.

https://docs.rs/crossbeam-epoch/0.9.18/crossbeam_epoch/struct.Guard.html#method.defer_destroy

There is no guarantee when exactly the destructor will be executed. The only guarantee is that it won’t be executed until all currently pinned threads get unpinned. In theory, the destructor might never run, but the epoch-based garbage collection will make an effort to execute it reasonably soon.

We have been struggling with this spec (notes) and added some mitigations. Now I am adding another mitigation via #384, but the new unit tests are a bit flaky.

gh-actions

Let me struggle a bit longer to see if I can stabilize them.

@tatsuya6502
Copy link
Member

I am not totally sure if I have stabilized the flaky tests, but I am going to merge #384 and will see how these jobs will go. If everything goes well, I will publish a new version moka@v0.12.4 to crates.io. Hopefully that will happen in the next few days.

Also I created the following issue to evaluate an alternatives of crossbeam-epoch:

Thanks again for reporting the issue!

@igorzi
Copy link
Author

igorzi commented Jan 20, 2024

Thanks!

@tatsuya6502
Copy link
Member

FYI, I just published moka@v0.12.4 to crates.io.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants