memory leak when redisContext has error

[fourier307]

We use redis as a cache. The redis logic part of our application is as follow:

redisContext* context = redisConnectWithTimeout(); redisReply* reply = NULL; while (1) { reply = (redisReply*)redisCommand(context, "GET %s", key); freeReplyObject(reply); // ... reply = (redisReply*)redisCommand(context, "SET %s %s EX %d", key, value, timeout); freeReplyObject(reply); // ... }

The application got serious memory leak. After a lot of efforts, we finally figured out the reason which was error using of hiredis. We did not check the state of redisContext before doing GET or SET. After the redis server closed the connection, we continued to GET and SET, then we got memory leak. The content of leaked memory is the formatted redis command, "GET %s" and "SET %s %s EX %d". As the size of our "value" was dozens of kilobyte, the memory leak speed is fast.

We noticed the description, "Once an error is returned the context cannot be reused and you should set up a new connection." Troubleshooting memory leak is not that easy. Would redisCommand function add a if-check to avoid memory leak when error?

void *redisCommand(redisContext *c, const char *format, ...) { // if (!c || c->err) return NULL; va_list ap; void *reply = NULL; va_start(ap,format); reply = redisvCommand(c,format,ap); va_end(ap); return reply; }

[fourier307]

The errstr of redisContext is "Server closed the connection".

[michael-grunder]

We do this in other parts of hiredis so it may be a reasonable change.

I'll take a look at it.