Memory leak in friso lexicon. (已解决)

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Run the test program with memory leak detection.

Looks like the 'e->_val' is not freed:

__STATIC_API__ void default_fdic_callback( hash_entry_t e ) 
{
    register uint_t i;
    friso_array_t syn;
    lex_entry_t lex = ( lex_entry_t ) e->_val;
    //free the lex->word
    FRISO_FREE( lex->word );    
    //free the lex->syn if it is not NULL
    if ( lex->syn != NULL ) {   
    syn = lex->syn;
    for ( i = 0; i < syn->length; i++ ) {
        FRISO_FREE( syn->items[i] );
    }
    free_array_list( syn );
    }
    // ====> Missing Free here!
    // FRISO_FREE(lex);
}

Original issue reported on code.google.com by mle...@gmail.com on 28 Jan 2014 at 8:23

[GoogleCodeExporter]

Also, if e->_val is not null here, it will leak as well:

FRISO_API void hash_put_mapping( 
    friso_hash_t _hash, 
    fstring key, 
    void * value ) 
{
    uint_t bucket = ( key == NULL ) ? 0 : hash( key, _hash->length );
    hash_entry_t e = *( _hash->table + bucket );

    //check the given key is already exists or not.
    for ( ; e != NULL; e = e->_next ) 
    {
    if ( key == e->_key 
        || ( key != NULL && e->_key != NULL 
            && strcmp( key, e->_key ) == 0 ) ) 
    {
            // ====> Missing Free here!
            // if (e->_val) FRISO_FREE(e->_val);
        e->_val = value;
        return;
    }
    }

    //put a new mapping into the hashtable.
    _hash->table[bucket] = new_hash_entry( key, value, _hash->table[bucket] );
    _hash->size++;

    //check the condition to rebuild the hashtable.
    if ( _hash->size >= _hash->threshold ) 
    rebuild_hash( _hash );

}

Original comment by mle...@gmail.com on 28 Jan 2014 at 8:30

[GoogleCodeExporter]

And finally, there seems to be multiple leaks caused by this line:

FRISO_API void friso_dic_load
...
_word = string_copy_heap( _buffer, strlen(_buffer) );
...

There seems to be missing multiple 'frees'.  One of them would be:

        if ( ! ( lex == __LEX_ECM_WORDS__ || lex == __LEX_CEM_WORDS__ )
            && strlen( _word ) > length ) {
            // ====> Missing Free here!
            // FRISO_FREE(_word);
            continue;
        }

Original comment by mle...@gmail.com on 28 Jan 2014 at 8:46

[GoogleCodeExporter]

Theses are only what I think the leaks comes from..  It might not be the right 
way to fix them.

Original comment by mle...@gmail.com on 28 Jan 2014 at 9:16

[GoogleCodeExporter]

In fact, I think the best way to fix this would be to call 
default_fdic_callback from 

FRISO_API void hash_put_mapping( 
    friso_hash_t _hash, 
    fstring key, 
    void * value ) 
{
    uint_t bucket = ( key == NULL ) ? 0 : hash( key, _hash->length );
    hash_entry_t e = *( _hash->table + bucket );

    //check the given key is already exists or not.
    for ( ; e != NULL; e = e->_next ) 
    {
    if ( key == e->_key 
        || ( key != NULL && e->_key != NULL 
            && strcmp( key, e->_key ) == 0 ) ) 
    {
            // ====> Missing Free here!
            //e->_key = key;
        //if ( _hash->fentry_func != NULL ) _hash->fentry_func(e);
        e->_val = value;
        return;
    }
    }

    //put a new mapping into the hashtable.
    _hash->table[bucket] = new_hash_entry( key, value, _hash->table[bucket] );
    _hash->size++;

    //check the condition to rebuild the hashtable.
    if ( _hash->size >= _hash->threshold ) 
    rebuild_hash( _hash );

}

We would need to modify hash struct:

typedef struct {
    uint_t length;
    uint_t size;
    float factor;
    uint_t threshold;
    hash_entry_t *table;
    // ===> fhash_callback_fn_t fentry_func;
} friso_hash_cdt;

and pass it at hash table creation like that:

dic[t] = new_hash_table(default_fdic_callback);

Original comment by mle...@gmail.com on 28 Jan 2014 at 9:23

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I backed home really early this year, and i live in the countryside without 
network condition.

Thanks for you feedback first and terribly sorry to responsed you so late.

I've put the source code of friso on http://git.oschina.net/lionsoul/friso, and 
you are really welcome to post you request and contribute code there!

Original comment by chenxin6...@gmail.com on 5 Feb 2014 at 2:45

[GoogleCodeExporter]

i've checked the code, the 1th and the 3th issue you posted will do cause 
memory leak.

As for the hash_put_mapping function in friso_lexicon.c, you can't just invoke 
function free to release the allocation of e->_val like you posted: "// if 
(e->_val) FRISO_FREE(e->_val);"

Cause the hash API was design for underlying use and you could never know the 
real struct of e->_val, i mean we could change it to return the e->_val, and 
let the developer to release it and it will be like this:

FRISO_API void *hash_put_mapping( 
    friso_hash_t _hash, 
    fstring key, 
    void * value ) 
{
    uint_t bucket = ( key == NULL ) ? 0 : hash( key, _hash->length );
    hash_entry_t e = *( _hash->table + bucket );
    void *oval = NULL;

    //check the given key is already exists or not.
    for ( ; e != NULL; e = e->_next ) 
    {
    if ( key == e->_key 
        || ( key != NULL && e->_key != NULL 
            && strcmp( key, e->_key ) == 0 ) ) 
    {
            oval = e->_val;
        e->_val = value;
        return oval;
    }
    }

    //put a new mapping into the hashtable.
    _hash->table[bucket] = new_hash_entry( key, value, _hash->table[bucket] );
    _hash->size++;

    //check the condition to rebuild the hashtable.
    if ( _hash->size >= _hash->threshold ) 
    rebuild_hash( _hash );

    return oval;
}

Original comment by chenxin6...@gmail.com on 5 Feb 2014 at 2:45

[GoogleCodeExporter]

Hi, I'm glad yo came back!

Indeed, we cannot call free on hash value.  However, as I mention in my post 
#4, you could call user defined free function:

if ( _hash->fentry_func != NULL ) _hash->fentry_func(e);

Original comment by mle...@gmail.com on 5 Feb 2014 at 2:59

[GoogleCodeExporter]

yat, that's a nice advice. Thanks...

Original comment by chenxin6...@gmail.com on 5 Feb 2014 at 10:22