-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
freeReplyObject usage #710
Comments
Well you don't have to call #include <stdio.h>
#include <hiredis/hiredis.h>
int main(int argc, const char **argv) {
redisContext *c = redisConnect("127.0.0.1", 6379);
for (int i = 0; i < 100; i++) {
redisReply *r = redisCommand(c, "PING %d", i);
// Only free the replies if passed an argument
if (argc > 1) {
if (r) freeReplyObject(r);
}
}
redisFree(c);
} If I run this program under $ valgrind --leak-check=full ./hrleak # No arguments, meaning we don't free replies
==32719== Memcheck, a memory error detector
==32719== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==32719== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==32719== Command: ./hrleak
==32719==
==32719==
==32719== HEAP SUMMARY:
==32719== in use at exit: 5,090 bytes in 200 blocks
==32719== total heap usage: 1,213 allocs, 1,013 frees, 20,656 bytes allocated
==32719==
==32719== 5,090 (4,800 direct, 290 indirect) bytes in 100 blocks are definitely lost in loss record 2 of 2
==32719== at 0x483AB35: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==32719== by 0x4874512: createReplyObject (hiredis.c:65)
==32719== by 0x4874512: createStringObject (hiredis.c:105)
==32719== by 0x487C77A: processBulkItem (read.c:339)
==32719== by 0x487C77A: processItem (read.c:481)
==32719== by 0x487C77A: redisReaderGetReply (read.c:576)
==32719== by 0x4876633: redisGetReplyFromReader (hiredis.c:838)
==32719== by 0x487671A: redisGetReply (hiredis.c:865)
==32719== by 0x4876A14: __redisBlockForReply (hiredis.c:970)
==32719== by 0x4876A14: redisvCommand (hiredis.c:980)
==32719== by 0x4876AE3: redisCommand (hiredis.c:986)
==32719== by 0x1090BE: main (hrleak.c:8)
==32719==
==32719== LEAK SUMMARY:
==32719== definitely lost: 4,800 bytes in 100 blocks
==32719== indirectly lost: 290 bytes in 100 blocks
==32719== possibly lost: 0 bytes in 0 blocks
==32719== still reachable: 0 bytes in 0 blocks
==32719== suppressed: 0 bytes in 0 blocks
==32719==
==32719== For counts of detected and suppressed errors, rerun with: -v
==32719== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) Running the program with an argument (meaning we will free replies) and there are no longer any leaks detected. $ valgrind --leak-check=full ./hrleak an_argument # Now we will call freeReplyObject
==695== Memcheck, a memory error detector
==695== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==695== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==695== Command: ./hrleak an_argument
==695==
==695==
==695== HEAP SUMMARY:
==695== in use at exit: 0 bytes in 0 blocks
==695== total heap usage: 1,213 allocs, 1,213 frees, 20,656 bytes allocated
==695==
==695== All heap blocks were freed -- no leaks are possible
==695==
==695== For counts of detected and suppressed errors, rerun with: -v
==695== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) As for your second question yes, hiredis still frees replies in asynchronous calls. I didn't write that part of the system so I'm not totally sure why we thought it was going to change in the future. Does that make sense? |
@michael-grunder , thank you for answer! but if it will be like this:
is it ok? i dont have to manually "zeromemory" of redisReply structure to safely recieve data?
so it works only in async calls and if i use redisConnect i need to freeReplyObject manually? |
Correct, you don't need to zero memory in the structure even if you call redisCommand in a loop. Hiredis will take care of allocating and setting the structure. #include <stdio.h>
#include <hiredis/hiredis.h>
int main(int argc, const char **argv) {
redisContext *c = redisConnect("127.0.0.1", 6379);
redisReply *r;
for (int i = 0; i < 10; i++) {
r = (redisReply*)redisCommand(c, "PING %d", i);
if (r) {
if (r->type == REDIS_REPLY_STRING) {
printf("redisReply[%p]->str[%p] = '%s'\n", r, r->str, r->str);
}
freeReplyObject(r);
}
}
redisFree(c);
} If you run this program you'll see the pointers changing in the loop, since hiredis is doing all of the allocation for you.
Right, If you're new to C it's important to point out that if you want to store a reply from redis somewhere else, you will need to duplicate that memory. Otherwise the memory will be freed by |
I'm going to close this issue because it's not a bug in hiredis but good luck with your program.! 😄 |
@h3xp1017 Hey, I just happened to look at one of your questions again and wanted to point something out. This program will leak memory: int main(int argc, const char **argv) {
redisContext *c = redisConnect("127.0.0.1", 6379);
redisReply *r;
for (int i = 0; i < 100; i++)
redisReply *r = (redisReply*)redisCommand(c, "PING %d", i);
freeReplyObject(r);
redisFree(c);
} You need to call #include <stdio.h>
#include <hiredis/hiredis.h>
int main(int argc, const char **argv) {
redisContext *c = redisConnect("127.0.0.1", 6379);
redisReply *r;
for (int i = 0; i < 100; i++) {
redisReply *r = (redisReply*)redisCommand(c, "PING %d", i);
if (r) freeReplyObject(r);
}
redisFree(c);
} |
well this is what i wanted to know, thank you =)
why this happen? i mean i use only one redisReply structure which is overwritten every time in the loop, why it leaks memory if its doesnt creates new objects in heap/stack? |
It does allocate memory on the heap. If you were to step into hiredis you'd see it executing a Here's a short program that does something similar but removes all of the complexity of hiredis #include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct exampleReply {
char *str;
} exampleReply;
char *createString(int count) {
char buf[1024], *str;
size_t len;
len = snprintf(buf, sizeof(buf), "Example reply %d", count);
str = malloc(len + 1);
memcpy(str, buf, len);
str[len] = '\0';
return str;
}
exampleReply *getReply(int count) {
exampleReply *r = malloc(sizeof(exampleReply));
r->str = createString(count);
return r;
}
void freeReply(exampleReply *reply) {
free(reply->str);
free(reply);
}
int main(void) {
for (int i = 0; i < 3; i++) {
exampleReply *r = getReply(i);
printf("exampleReply[%p]->str[%p] = '%s'\n", r, r->str, r->str);
freeReply(r);
}
} Notice how I highly recommend using programs like valgrind or clang's asan when learning to help you find things like memory leaks or invalid memory reads. Even programmers who've been using C for decades use these tools. |
why do i actually have to use freeReplyObject? what this function actually do? for example this code works even without freeReplyObject:
redReply = (redisReply*)redisCommand(c, "PING"); // invoke PING command & return pointer to redisReply structure
printf("PING: %s\n", redReply->str);
//freeReplyObject(redReply); // free reply structure
redReply = (redisReply*)redisCommand(c, "GET testkey");
printf("testkey: %s\n", redReply->str);
is this information still relevant?
The text was updated successfully, but these errors were encountered: