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

Segfault upon destruction using hashmap of hashmaps #2

Open
charlesgregory opened this issue Apr 17, 2020 · 7 comments
Open

Segfault upon destruction using hashmap of hashmaps #2

charlesgregory opened this issue Apr 17, 2020 · 7 comments
Labels
bug Something isn't working

Comments

@charlesgregory
Copy link

In trying to make a hashmap of hashmaps using khash.d, I run into the issue of a segfault when using the require function.
Segfault happens here as a double free (I think):

dklib/source/dklib/khash.d

Lines 145 to 147 in 8b3fa92

kfree(cast(void*) this.keys);
kfree(cast(void*) this.flags);
kfree(cast(void*) this.vals);

The initval is an empty hashmap.
Example:

auto kmers = khash!(string, khash!(string, int))();

// for loop {

    // if not present create empty hash
    // initval is empty hashmap
    kmers.require(kmer1, khash!(string, int)());

    // if not present make count zero
    kmers[kmer1].require(kmer2, 0);
  
    kmers[kmer1][kmer2]++;

//}

The fix I found was by removing the free statements in the destructor. My theory is that D's automatic destructor frees the pointers as soon as they are out of scope. Or the other theory is that the empty hashmaps are freed before ever having the pointers set to any memory and are therefore the pointers are null or invalid.

@jblachly
Copy link
Member

jblachly commented Aug 26, 2020

Code LGTM, probably you are seeing a double-free since I did not disable copies.
If you @disable this(this) and then attempt your test you should see a compilation failure indicating that a copy is not allowed. Can you confirm this?

@jblachly
Copy link
Member

If you confirm this is the problem I will either disable copies or add copy constructor since that is a relatively new feature in Dlang

@charlesgregory
Copy link
Author

Yes disabling copies provides me with a long list of compilation errors.

@jblachly
Copy link
Member

@charlesgregory can you please provide example code of how you are using nested hashmaps to better understand how we can work around this? I am in favor of disabling copies, but we could potentially instead do refcounting

@charlesgregory
Copy link
Author

auto kmers = khash!(string, khash!(string, int))();

 for loop {
    string kmer1;
    string kmer2;

    // if kmer1 not present create empty hashmap
    // initval is empty hashmap
    kmers.require(kmer1, khash!(string, int)()); // <- segfault when kfrees are commented out

    // if kmer2 not present make count zero
    kmers[kmer1].require(kmer2, 0);
  
    kmers[kmer1][kmer2]++;

}

This could potentially be solved by having special compile time checks for a value type that is another khash instance. Might be difficult.

@jblachly
Copy link
Member

Yeah you are constructing the object in the parameter position and it is copied into the function.

@jblachly
Copy link
Member

Changing the ctor function signature value parameter to include auto ref may fix this although ia m not sure whether should do that or write copy constructor (or both)

@jblachly jblachly added the bug Something isn't working label Aug 30, 2020
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

No branches or pull requests

2 participants