{"action":"created","issue":{"url":"https://api.github.com/repos/redis/hiredis/issues/367","labels_url":"https://api.github.com/repos/redis/hiredis/issues/367/labels{/name}","comments_url":"https://api.github.com/repos/redis/hiredis/issues/367/comments","events_url":"https://api.github.com/repos/redis/hiredis/issues/367/events","html_url":"https://github.com/redis/hiredis/pull/367","id":109134523,"number":367,"title":"Fix libuv adapter to handle polling errors","user":{"login":"rangan337","id":8302774,"avatar_url":"https://avatars.githubusercontent.com/u/8302774?v=3","gravatar_id":"","url":"https://api.github.com/users/rangan337","html_url":"https://github.com/rangan337","followers_url":"https://api.github.com/users/rangan337/followers","following_url":"https://api.github.com/users/rangan337/following{/other_user}","gists_url":"https://api.github.com/users/rangan337/gists{/gist_id}","starred_url":"https://api.github.com/users/rangan337/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rangan337/subscriptions","organizations_url":"https://api.github.com/users/rangan337/orgs","repos_url":"https://api.github.com/users/rangan337/repos","events_url":"https://api.github.com/users/rangan337/events{/privacy}","received_events_url":"https://api.github.com/users/rangan337/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-09-30T16:51:56Z","updated_at":"2015-10-01T04:04:03Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/redis/hiredis/pulls/367","html_url":"https://github.com/redis/hiredis/pull/367","diff_url":"https://github.com/redis/hiredis/pull/367.diff","patch_url":"https://github.com/redis/hiredis/pull/367.patch"},"body":"## Introduction\r\nCurrently, when an async connection is attached to a *libuv* event loop using the libuv adapter, a libuv poll handle is initialized using the connection's file descriptor. \r\n\r\nAll subsequent async reads / writes are done by polling the file descriptor associated with the poll handle (using *uv_poll_start()* and *uv_poll_stop()*). \r\n\r\nWhen an event is available, the **redisLibuvPoll()** function is invoked as the callback with **status** set to 0 and the detected event on the **events** parameter.\r\n\r\n## Issue\r\nIn the event of a polling error, the **redisLibuvPoll()** callback function is invoked with **status < 0** (exact error status depends on the reason the polling failed). Currently, this function returns if an error status is encountered without taking any action. \r\n\r\nThis can be problematic in various error scenarios, two of which are detailed below -\r\n\r\n### Scenario 1 - When a connection fails \r\nThe steps involved in creating an async connection are -\r\n1. Call *redisAsyncConnect()*\r\n2. Call *redisLibuvAttach()* to attach the async connection to the event loop\r\n3. Set connect / disconnect callbacks for the async connection using *redisAsyncSetConnectCallback()* and *redisAsyncSetDisconnectCallback()*\r\n4. Run the event loop to check if the connection was successful or not\r\n\r\nIn step 4, if the connection was successful, the callback set in *redisAsyncSetConnectCallback()* is called.\r\n\r\n**However** if the connection is not successful, an error is generated in polling the file descriptor and no action is taken.\r\n\r\n### Scenario 2 - Server crashes and the connection is broken\r\nIf the Redis server crashes, the connection breaks resulting in a polling error. Now subsequent Redis commands issued using the <b>redisAsyncCommand()</b> family of functions would internally follow the steps below - \r\n1. Call *__redisAsyncCommand()* helper function \r\n2. Call \r\n```\r\nEL_ADD_WRITE(ac);\r\n```\r\nwhich expands to calling *redisLibuvAddWrite()*\r\n3. Call *redisLibuvPoll()* which returns without taking any action\r\n\r\nAs a result, their callbacks would never be invoked and writes will get queued up indefinitely on this broken connection **unless** the connection is disconnected by a call to some other function in hiredis.c (which you really can't rely on to happen in all use cases)\r\n\r\n## Fix / Resolution\r\nTo address the above scenarios and similar ones, if a polling error is encountered, we mark the *err* and *errstr* fields with appropriate error flag and error message respectively. \r\n\r\nWe also invoke *redisAsyncHandleRead()* so that \r\n1. If a connection is not successful, *__redisAsyncHandleConnect()* can be called\r\n2. If any other error occurs on a valid connection, (like scenario 2 above) *redisBufferRead()* would return REDIS_ERR and *__redisAsyncDisconnect()* will be called.\r\n\r\nNote that either *redisAsyncHandleRead()* or *redisAsyncHandleWrite()* could be called when an error was encountered, but I went with *redisAsyncHandleRead()* as it seemed to be cleaner (no *done* variable)\r\n\r\n## Examples\r\n### Scenario 1 - When a connection fails\r\nExample Gist Link - https://gist.github.com/rangan337/11a5e893ec41977b901b \r\n\r\nThis is a simple example that assumes that a Redis server is not running on the PORT it is querying. \r\n\r\n* Prior to the PR, no connection errors are reported. \r\n* With the PR, the error that the client failed to connect to the server is reported.\r\n\r\n\r\n### Scenario 2 - Server crashes and the connection is broken\r\nExample Gist Link (With Sample Output) - https://gist.github.com/rangan337/77da6f7069d0fb29d443\r\n\r\nThis example basically : \r\n\r\n1. Creates an async connection to a Redis server and attaches it to an event loop (running on its own thread)\r\n2. Following that, the server is populated with some data. \r\n3. At this point, on a parallel thread a **DEBUG SEGFAULT** is sent to crash the server.\r\n4. While 3 is going on, in parallel requests are sent over the async connection by a set of 20 or so threads. Note that - \r\n  * While 20 *run command* events are fired on the event loop, as there is no guarantee that each *fire* will trigger the event handler, its quite possible that < 20 commands are actually sent to the server\r\n  * As this is in parallel with the segfault, some commands will be executed before and some after the segfault\r\n5. Sleep the main thread for 2 seconds to allow all the other threads to do their job\r\n6. Print the number of pending queued callbacks on the async connection. \r\n  * **Prior to the PR,** this value would be >=0. \r\n  * **After the PR,** this value will always be 0 (as when the connection is disconnected all pending callbacks are executed with a NULL reply)\r\n7. Send another wave of parallel requests over the connection to the server. \r\n  * **Prior to the PR,** this will silently queue up the callbacks on the broken connection. \r\n  * Note that nothing is done in the **after PR** case as when the connection is disconnected in the step above, it is freed and set to NULL.\r\n8. Lastly, print out the queued callback count (this is just to validate that they get queued up prior to the PR). \r\n\r\nNote that for this example, the gist also has the sample output of a run attached."},"comment":{"url":"https://api.github.com/repos/redis/hiredis/issues/comments/144614201","html_url":"https://github.com/redis/hiredis/pull/367#issuecomment-144614201","issue_url":"https://api.github.com/repos/redis/hiredis/issues/367","id":144614201,"user":{"login":"rangan337","id":8302774,"avatar_url":"https://avatars.githubusercontent.com/u/8302774?v=3","gravatar_id":"","url":"https://api.github.com/users/rangan337","html_url":"https://github.com/rangan337","followers_url":"https://api.github.com/users/rangan337/followers","following_url":"https://api.github.com/users/rangan337/following{/other_user}","gists_url":"https://api.github.com/users/rangan337/gists{/gist_id}","starred_url":"https://api.github.com/users/rangan337/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rangan337/subscriptions","organizations_url":"https://api.github.com/users/rangan337/orgs","repos_url":"https://api.github.com/users/rangan337/repos","events_url":"https://api.github.com/users/rangan337/events{/privacy}","received_events_url":"https://api.github.com/users/rangan337/received_events","type":"User","site_admin":false},"created_at":"2015-10-01T04:04:03Z","updated_at":"2015-10-01T04:04:03Z","body":"Thanks @badboy !\r\n\r\nI have also created an example illustrating scenario 2.\r\n\r\nI have updated the PR description with an *Examples* section with the gists attached so that all the info is in one place. Let me know if you need any more info here. "}}