{"action":"created","issue":{"url":"https://api.github.com/repos/eddelbuettel/rcppredis/issues/20","labels_url":"https://api.github.com/repos/eddelbuettel/rcppredis/issues/20/labels{/name}","comments_url":"https://api.github.com/repos/eddelbuettel/rcppredis/issues/20/comments","events_url":"https://api.github.com/repos/eddelbuettel/rcppredis/issues/20/events","html_url":"https://github.com/eddelbuettel/rcppredis/issues/20","id":118358566,"number":20,"title":"R crash on Redis server loss","user":{"login":"richfitz","id":1558093,"avatar_url":"https://avatars.githubusercontent.com/u/1558093?v=3","gravatar_id":"","url":"https://api.github.com/users/richfitz","html_url":"https://github.com/richfitz","followers_url":"https://api.github.com/users/richfitz/followers","following_url":"https://api.github.com/users/richfitz/following{/other_user}","gists_url":"https://api.github.com/users/richfitz/gists{/gist_id}","starred_url":"https://api.github.com/users/richfitz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/richfitz/subscriptions","organizations_url":"https://api.github.com/users/richfitz/orgs","repos_url":"https://api.github.com/users/richfitz/repos","events_url":"https://api.github.com/users/richfitz/events{/privacy}","received_events_url":"https://api.github.com/users/richfitz/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/eddelbuettel/rcppredis/labels/bug","name":"bug","color":"fc2929"},{"url":"https://api.github.com/repos/eddelbuettel/rcppredis/labels/help%20welcome","name":"help welcome","color":"c7def8"},{"url":"https://api.github.com/repos/eddelbuettel/rcppredis/labels/wontfix","name":"wontfix","color":"ffffff"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-11-23T10:59:59Z","updated_at":"2015-11-23T19:02:02Z","closed_at":null,"body":"Minimal, contrived, example:\r\n\r\n```r\r\nsystem(\"redis-server --port 7777\", wait=FALSE)\r\nr <- new(RcppRedis::Redis, \"localhost\", 7777)\r\nr$ping()\r\nr$exec(\"SHUTDOWN\")\r\n```\r\n\r\nwill crash R with\r\n\r\n```\r\n> r$exec(\"SHUTDOWN\")\r\n[15435] 23 Nov 10:54:23.121 # User requested shutdown...\r\n[15435] 23 Nov 10:54:23.122 # Redis is now ready to exit, bye bye...\r\n\r\n *** caught segfault ***\r\naddress (nil), cause 'memory not mapped'\r\n\r\nTraceback:\r\n 1: .External(list(name = \"CppMethod__invoke_notvoid\", address = <pointer: 0x2fab870>,     dll = list(name = \"Rcpp\", path = \"/home/rich/lib/R/library/Rcpp/libs/Rcpp.so\",         dynamicLookup = TRUE, handle = <pointer: 0x35d8390>,         info = <pointer: 0x7fc132594c40>), numParameters = -1L),     <pointer: 0x271b430>, <pointer: 0x362b4a0>, .pointer, ...)\r\n 2: r$exec(\"SHUTDOWN\")\r\n\r\nPossible actions:\r\n1: abort (with core dump, if enabled)\r\n2: normal R exit\r\n3: exit R without saving workspace\r\n4: exit R saving workspace\r\n```\r\n\r\nSeen on various flavours of OSX and Linux.  The gdb backtrace is:\r\n\r\n```\r\n(gdb) bt\r\n#0  Redis::extract_reply (this=0xef74a0, reply=0x0) at Redis.cpp:76\r\n#1  0x00007ffff2d54884 in Redis::exec (this=0xef74a0, cmd=...) at Redis.cpp:157\r\n#2  0x00007ffff2d531e3 in Rcpp::CppMethod1<Redis, SEXPREC*, std::string>::operator() (this=this@entry=0x1d6dcd0, object=0xef74a0, args=args@entry=0x7fffffffbde0)\r\n    at /home/rich/lib/R/library/Rcpp/include/Rcpp/module/Module_generated_CppMethod.h:111\r\n#3  0x00007ffff2d5b25f in Rcpp::class_<Redis>::invoke_notvoid (this=<optimised out>, method_xp=<optimised out>, object=0x2007a60, args=0x7fffffffbde0, \r\n    nargs=<optimised out>) at /home/rich/lib/R/library/Rcpp/include/Rcpp/module/class.h:234\r\n#4  0x00007ffff348b1ec in CppMethod__invoke_notvoid (args=<optimised out>) at Module.cpp:220\r\n#5  0x00007ffff78f1161 in ?? () from /usr/lib/R/lib/libR.so\r\n#6  0x00007ffff79369db in Rf_eval () from /usr/lib/R/lib/libR.so\r\n#7  0x00007ffff7938b60 in ?? () from /usr/lib/R/lib/libR.so\r\n#8  0x00007ffff79367e3 in Rf_eval () from /usr/lib/R/lib/libR.so\r\n#9  0x00007ffff7937b6f in Rf_applyClosure () from /usr/lib/R/lib/libR.so\r\n#10 0x00007ffff79365bf in Rf_eval () from /usr/lib/R/lib/libR.so\r\n#11 0x00007ffff795da32 in Rf_ReplIteration () from /usr/lib/R/lib/libR.so\r\n#12 0x00007ffff795dd81 in ?? () from /usr/lib/R/lib/libR.so\r\n#13 0x00007ffff795de34 in run_Rmainloop () from /usr/lib/R/lib/libR.so\r\n#14 0x00000000004007eb in main ()\r\n#15 0x00007ffff7261ec5 in __libc_start_main (main=0x4007d0 <main>, argc=1, argv=0x7fffffffde78, init=<optimised out>, fini=<optimised out>, \r\n    rtld_fini=<optimised out>, stack_end=0x7fffffffde68) at libc-start.c:287\r\n#16 0x000000000040081b in _start ()\r\n```\r\n\r\nA fix would be to add something like:\r\n\r\n```\r\n  if (reply == NULL) {\r\n    error(\"Failure communicating with the Redis server\");\r\n  }\r\n```\r\n\r\nafter every `redisCommand` / `redisCommandArgv` in src/Redis.cpp\r\n\r\nWhile the above is through Redis server shutdown the same thing happens if the connection is lost (e.g. client and server on different machines on different hosts and a loss of the link between).  I have confirmed this with toxiproxy."},"comment":{"url":"https://api.github.com/repos/eddelbuettel/rcppredis/issues/comments/159028934","html_url":"https://github.com/eddelbuettel/rcppredis/issues/20#issuecomment-159028934","issue_url":"https://api.github.com/repos/eddelbuettel/rcppredis/issues/20","id":159028934,"user":{"login":"drknexus","id":1847158,"avatar_url":"https://avatars.githubusercontent.com/u/1847158?v=3","gravatar_id":"","url":"https://api.github.com/users/drknexus","html_url":"https://github.com/drknexus","followers_url":"https://api.github.com/users/drknexus/followers","following_url":"https://api.github.com/users/drknexus/following{/other_user}","gists_url":"https://api.github.com/users/drknexus/gists{/gist_id}","starred_url":"https://api.github.com/users/drknexus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/drknexus/subscriptions","organizations_url":"https://api.github.com/users/drknexus/orgs","repos_url":"https://api.github.com/users/drknexus/repos","events_url":"https://api.github.com/users/drknexus/events{/privacy}","received_events_url":"https://api.github.com/users/drknexus/received_events","type":"User","site_admin":false},"created_at":"2015-11-23T19:02:02Z","updated_at":"2015-11-23T19:02:02Z","body":"Well, the ping will tell you that it is up at the time of the ping, but not that it was actually up/connected correctly at the time of your request.  I was thinking instead of wrapping redisCommand and redisCommandArgv with a safety check that a NULL wasn't received as the reply.  It would mean refactoring the code to use the NULL safe versions of those functions.\r\n\r\nI quickly banged out the following:\r\n\r\n```\r\n    void nullReplyCheck(redisReply *reply) {\r\n        if (reply == NULL) {\r\n            Rcpp::stop(\"Recieved NULL reply; potential connection loss with Redis\");\r\n        }\r\n    }\r\n    \r\n    void *redisCommandNULLSafe(redisContext *c, const char *format, ...) {\r\n        va_list myargs;\r\n        va_start(myargs, format);\r\n        \r\n        redisReply *reply = \r\n            static_cast<redisReply*>(redisvCommand(c, format, myargs));\r\n        nullReplyCheck(reply);\r\n        return reply;\r\n    }\r\n    \r\n    void *redisCommandArgvNULLSafe(redisContext *c, int argc, const char **argv, const size_t *argvlen) {\r\n        redisReply *reply = \r\n            static_cast<redisReply*>(redisCommandArgv(c, argc, argv, argvlen));\r\n        nullReplyCheck(reply);\r\n        return reply;\r\n    }\r\n```"}}