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

Memory leak? Inserted value will never be dropped? #23

Closed
tatsuya6502 opened this issue May 29, 2021 · 3 comments
Closed

Memory leak? Inserted value will never be dropped? #23

tatsuya6502 opened this issue May 29, 2021 · 3 comments

Comments

@tatsuya6502
Copy link

  • cht 0.4.1
  • Rust 1.52.1 (9bc8c42bb 2021-05-09)

It seems cht::HashMap will never drop inserted values when they are removed from the map or replaced by new values. Instead, a clone of a value will be created and the clone will be dropped. The following code will illustrate the problem.

use cht::HashMap;

struct A(String);

impl Clone for A {
    fn clone(&self) -> Self {
        println!("Cloned    A({})", self.0);
        A(format!("a clone of A({})", self.0))
    }
}

impl Drop for A {
    fn drop(&mut self) {
        println!("Dropped   A({}).", self.0)
    }
}

fn main() {
    let cht = HashMap::with_capacity(1);
    println!("Inserting A(0)");
    cht.insert(0, A("0".into()));
    println!("Removing  A(0)");
    cht.remove(&0).unwrap();
    println!("Inserting A(1)");
    cht.insert(0, A("1".into()));
    println!("Inserting A(2)");
    cht.insert(0, A("2".into()));
    println!("Inserting A(3)");
    cht.insert(0, A("3".into()));
    println!("Inserting A(4)");
    cht.insert(0, A("4".into()));
    println!("Inserting A(5)");
    cht.insert(0, A("5".into()));
    println!("Removing  A(5)");
    cht.remove(&0).unwrap();
    println!("Inserting A(6)");
    cht.insert(0, A("6".into()));
    println!("Dropping  cht (len: {}, capacity:{})", cht.len(), cht.capacity());
    std::mem::drop(cht);
}
$ cargo run
...
Inserting A(0)
Removing  A(0)
Cloned    A(0)
Dropped   A(a clone of A(0)).
Inserting A(1)
Inserting A(2)
Cloned    A(1)
Dropped   A(a clone of A(1)).
Inserting A(3)
Cloned    A(2)
Dropped   A(a clone of A(2)).
Inserting A(4)
Cloned    A(3)
Dropped   A(a clone of A(3)).
Inserting A(5)
Cloned    A(4)
Dropped   A(a clone of A(4)).
Removing  A(5)
Cloned    A(5)
Dropped   A(a clone of A(5)).
Inserting A(6)
Dropping  cht (len: 1, capacity:1)
@tatsuya6502
Copy link
Author

Maybe bucket::defer_destroy_bucket (source) is not working as expected? (although I see nothing wrong in the code)

I am using crossbeam-epoch 0.8.2.

$ cargo tree
cht-test v0.1.0 (... /cht-test)
└── cht v0.4.1
    ├── ahash v0.3.8
    │   └── const-random v0.1.13
    │       ├── const-random-macro v0.1.13 (proc-macro)
    │       │   ├── getrandom v0.2.3
    │       │   │   ├── cfg-if v1.0.0
    │       │   │   └── libc v0.2.95
    │       │   ├── lazy_static v1.4.0
    │       │   ├── proc-macro-hack v0.5.19 (proc-macro)
    │       │   └── tiny-keccak v2.0.2
    │       │       └── crunchy v0.2.2
    │       └── proc-macro-hack v0.5.19 (proc-macro)
    ├── crossbeam-epoch v0.8.2
    │   ├── cfg-if v0.1.10
    │   ├── crossbeam-utils v0.7.2
    │   │   ├── cfg-if v0.1.10
    │   │   └── lazy_static v1.4.0
    │   │   [build-dependencies]
    │   │   └── autocfg v1.0.1
    │   ├── lazy_static v1.4.0
    │   ├── maybe-uninit v2.0.0
    │   ├── memoffset v0.5.6
    │   │   [build-dependencies]
    │   │   └── autocfg v1.0.1
    │   └── scopeguard v1.1.0
    │   [build-dependencies]
    │   └── autocfg v1.0.1
    └── num_cpus v1.13.0
        └── libc v0.2.95

@tatsuya6502
Copy link
Author

I read the document about crossbeam_epoch::Guard::defer_unchecked. And now I think this is an expected behavior.

This method first stores f into the thread-local (or handle-local) cache. If this cache becomes full, some functions are moved into the global cache. At the same time, some functions from both local and global caches may get executed in order to incrementally clean up the caches as they fill up.

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

To confirm, I will need to store more data so that crossbeam-epoch's cache will be filled up and drop some values.

@tatsuya6502
Copy link
Author

To confirm, I will need to store more data so that crossbeam-epoch's cache will be filled up and drop some values.

I wrote another code and verified that majority of the replaced values were dropped. I believe crossbeam-epoch and cht are working as expected.

use cht::HashMap;
use std::sync::Arc;

fn main() {
    const CAP: usize = 10;
    const N_INSERTS: usize = CAP * 1_000;

    let cht = HashMap::with_capacity(CAP);
    let mut handles = Vec::with_capacity(N_INSERTS);

    for _ in 0..N_INSERTS {
        let v = Arc::new("*".to_string());
        cht.insert(0, Arc::clone(&v));
        handles.push(Arc::downgrade(&v));
    }

    let dropped = handles
        .into_iter()
        .filter(|v| v.strong_count() == 0)
        .count();

    println!("inserted: {}, dropped: {}", N_INSERTS, dropped);
}
$ cargo run --release --bin main2
...
inserted: 10000, dropped: 9832

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant