Search code examples
cvalgrindhiredis

redis freeReplyObject not freeing memory

Here is the code:

redisReply * reply;
char * stats = get_key(key, reply);
freeReplyObject( reply );

get_key is a function in a separate header file:

char * get_key(const char* key, redisReply * reply)
{
    reply = redisCommand(rc, "GET %s", key);
    if (reply->type != REDIS_REPLY_ERROR) {
        return reply->str;
    } else {
        return "ERROR";
    }
}

and Valgrind says this about it:

==24846== 2,592 bytes in 54 blocks are definitely lost in loss record 63 of 85
==24846==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24846==    by 0x4E3E342: createReplyObject (hiredis.c:64)
==24846==    by 0x4E3E342: createNilObject (hiredis.c:179)
==24846==    by 0x4E469FE: processBulkItem (read.c:267)
==24846==    by 0x4E469FE: processItem (read.c:407)
==24846==    by 0x4E469FE: redisReaderGetReply (read.c:503)
==24846==    by 0x4E406E3: redisGetReplyFromReader (hiredis.c:863)
==24846==    by 0x4E407CA: redisGetReply (hiredis.c:890)
==24846==    by 0x409DEE: RedisCluster::HiredisCommand<RedisCluster::Cluster<redisContext, RedisCluster::DefaultContainer<redisContext> > >::processHiredisCommand(redisContext*) (in /bucket)
==24846==    by 0x408E3A: RedisCluster::HiredisCommand<RedisCluster::Cluster<redisContext, RedisCluster::DefaultContainer<redisContext> > >::process() (in /bucket)
==24846==    by 0x408818: RedisCluster::HiredisCommand<RedisCluster::Cluster<redisContext, RedisCluster::DefaultContainer<redisContext> > >::Command(RedisCluster::Cluster<redisContext, RedisCluster::DefaultContainer<redisContext> >*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, char const*, ...) (in /bucket)
==24846==    by 0x405104: get_key(char const*, redisReply*) (in /bucket)
==24846==    by 0x406013: main (in /bucket)

I'm guessing it has something to do with the way I pass the reply pointer around, but I can't really tell what I'm doing wrong.

Solution

  • As said in the first comment, you have a problem of indirection.

    If I rewrite your code but inline get_key and rename the formal argument of get_key from reply to replyLocal and just concentrate on the reply variable we get

    redisReply * reply;
//char * stats = get_key(key, reply);
{
// this is what happens in the function call, reply is copied in to replyLocal
char* replyLocal = reply;
// and this is in the body of the function
replyLocal = redisCommand(rc, "GET %s", key);
// if and return ignored
}

    I hope that it is clear that the assignment to replyLocal does not modify reply. This means that when you try to free the memory it does not work as hoped.

    If you change the signature of get_key to

    char * get_key(const char* key, redisReply ** reply)

    and adapt the body to the extra level of indirection, starting with

    *reply = redisCommand(rc, "GET %s", key);

    and the call then becomes

    char * stats = get_key(key, &reply);

    Applying the same sort of inlining as above we have

    redisReply * reply;
//char * stats = get_key(key, &reply);
{
// this is what happens in the function call,
// ** the address of ** reply is copied in to replyLocal
char** replyLocal = &reply;
// and this is in the body of the function
*replyLocal = redisCommand(rc, "GET %s", key);
// if and return ignored
}

    This now modifies what replyLocal points to so it also modifies reply