search

shinberg / cpp-hiredis-cluster

c++ cluster wrapper for hiredis with async and unix sockets features
BSD 3-Clause "New" or "Revised" License
65 stars 26 forks source link

invalid std::shared_ptr<redisReply> from AltCommand #7

Closed vin-d closed 7 years ago

vin-d commented 7 years ago

Hi, I have just begun to use cpp-hiredis-cluster with synchronous calls and I'm experiencing crashes linked to reply = RedisCluster::HiredisCommand<>::AltCommand(redisClusterCtx, key.c_str(), "SET %s 0 NX", key.c_str());

it seems that following disconnection with a server from the cluster, AltCommand() passes an non-allocated redisReply object inside the std::shared_ptr<redisReply> object.

At the end of my function (end of scope of the the shared_ptr), the _shared_ptr's pointed object_ deleter is called and SIGABRT is triggered in HiredisCommand::deleteReply on freeReplyObject(reply)

Have you ever experienced anything like that ? I see that while doing tests, by shuting down some redis servers.

vin-d commented 7 years ago

Ok, the problem is the one exposed by aboutbus is his Pull Request https://github.com/shinberg/cpp-hiredis-cluster/pull/3 the root issue is that when a command is sent with hiredis, redisContext.err !=0 should first be checked before considering that the redisReply is valid.

Note that I've spent some time on that because the merge from aboutbus's fork was not made (the pull request is still open). Also, if this as been fixed in commit 4532983 for asynchronous calls (I say "if" because I haven't looked into asynchronous implementation of the library), it is still missing for the synchronous call.

shinberg commented 7 years ago

https://github.com/shinberg/cpp-hiredis-cluster/commit/f4e8f5679632a397ff927731d2df3d93e0c0bb5b fixed here