async api core dump when in multi thread sisuation

[jackycchen]

1. Env:
   hiredis: 0.14.0
   libevent: 2.0.21-stable

2. Situation:
   In my program, use redisAsycnCommand to send redis command(e.j. set/get), multi threads works separately, that means every worker threads holds a independent redisAsyncContext object.
   As we all known, the libevent function "event_base_dispatch" is a block api, which a thread call this api will block the thread to execute conitinue until there'are no events to proccess or call exit function to terminate.
   So, I create a independent thread to start the event_base_dispatch, and i can run stable in a low pressure request, but encountered segment fault when lots of requests call.
   My demo code is :

redisAsyncContext *c = redisAsyncConnect("127.0.0.1", 6379);
redisLibeventAttach(c, base);
redisAsyncSetConnectCallback(c, connectCallback);
redisAsyncSetDisconnectCallback(c, disconnectCallback);
pthread_t tid;
pthread_create(&tid, NULL, raw_async_thread_func, (void*)param);
pthread_create(&ttid, NULL, start_event_base_func, (void*)base);

3. Question:
   I review the hiredis code, and found that every async api operate the buf of redisAsyncContext, so when in the above sisuation, two thread may modify the buf at the same time, and the sds is not thread safty object, so segment fault can happen.
   The demo of the official wiki is:

struct event_base *base = event_base_new();

    redisAsyncContext *c = redisAsyncConnect("127.0.0.1", 6379);
    if (c->err) {
        /* Let *c leak for now... */
        printf("Error: %s\n", c->errstr);
        return 1;
    }
    redisLibeventAttach(c,base);
    redisAsyncSetConnectCallback(c,connectCallback);
    redisAsyncSetDisconnectCallback(c,disconnectCallback);
    redisAsyncCommand(c, NULL, NULL, "SET key %b", argv[argc-1], strlen(argv[argc-1]));
    redisAsyncCommand(c, getCallback, (char*)"end-1", "GET key");
    event_base_dispatch(base);

but, this cannot recieve any other command using redisAsyncCommand, because of event_base_dispatch block current thread to run continue.
So this is unreasonable usage code, I think we want to send redis commands to server work with event loop together, that means one worker produce task and libevent consume it.
I think if the we add lock before operate the buf object, the sdscatlen will not cause segment fault again.

some related issue list :

• coredump at srscatlen #460
• core dump on calling sdsMakeRoomFor #493
• Problem in sdsMakeRoomFor() in sds.c #221  #480
• async.c:451: redisProcessCallbacks: Assertion `(c->flags & 0x20 || c->flags & 0x40)' failed. #490

Mybe someone encounter the same sisuation, really hope can share the correct code
Thanks.

[mnunberg]

Let me understand this.. you want to be able to send commands from different threads, while having it wait for replies on a blocking thread?

[rajanarayan92]

Hi @jackycchen ,

Even I have faced this exact situation, no problem if the load is less. But when the requests are bombarded it dumps core due the reasons you mentioned.

Please let me know do you have any solution to this?
Need some inputs on design approach to properly integrate hiredis async into real world application.
Is it unsafe to use async API in such situation?

Thanks & Regards.

[michael-grunder]

Hi everybody,

This isn't really a bug in hiredis. The redisContext struct isn't thread-safe, so you can't manipulate the buffer in multiple threads without synchronization code.

We use hiredis in a highly threaded way in production, but our use case allows for no sharing, which makes it much easier.

Perhaps one of the thread-safe wrappers of hiredis could work? I've not used it but I found this one

[DGuco]

if i add a lock to every signle redisAsyncContext,every time i operate redisAsyncContext i lock then unlock, it will work?

[michael-grunder]

You can always try, but I don't suggest that strategy. My suggestion would be to not share redisContext structures across threads to avoid headaches.

There are also hiredis wrappers that have been made thread safe although I've not used them myself.

Closing as I don't think there's a hiredis bug here.