{"action":"created","issue":{"url":"https://api.github.com/repos/ellzey/libevhtp/issues/184","labels_url":"https://api.github.com/repos/ellzey/libevhtp/issues/184/labels{/name}","comments_url":"https://api.github.com/repos/ellzey/libevhtp/issues/184/comments","events_url":"https://api.github.com/repos/ellzey/libevhtp/issues/184/events","html_url":"https://github.com/ellzey/libevhtp/issues/184","id":105529891,"number":184,"title":"When `max_keepalive_requests` is set, connection didn't not close properly.","user":{"login":"cuber","id":1525381,"avatar_url":"https://avatars.githubusercontent.com/u/1525381?v=3","gravatar_id":"","url":"https://api.github.com/users/cuber","html_url":"https://github.com/cuber","followers_url":"https://api.github.com/users/cuber/followers","following_url":"https://api.github.com/users/cuber/following{/other_user}","gists_url":"https://api.github.com/users/cuber/gists{/gist_id}","starred_url":"https://api.github.com/users/cuber/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cuber/subscriptions","organizations_url":"https://api.github.com/users/cuber/orgs","repos_url":"https://api.github.com/users/cuber/repos","events_url":"https://api.github.com/users/cuber/events{/privacy}","received_events_url":"https://api.github.com/users/cuber/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-09-09T06:32:07Z","updated_at":"2015-09-09T06:38:22Z","closed_at":null,"body":"If the request was paused and later resumed, the `max_keepalive_requests` won't work as expected.\r\n\r\nThe problem can be reproduced as below.\r\n\r\n```c\r\n#include <stdio.h>\r\n#include <stdlib.h>\r\n#include <string.h>\r\n#include <stdint.h>\r\n#include <errno.h>\r\n#include <evhtp.h>\r\n\r\nstruct event * e = NULL;\r\n\r\nvoid\r\neventcb(evutil_socket_t fd, short which, void * ptr)\r\n{\r\n  evhtp_request_t * req = ptr;\r\n  evbuffer_add(req->buffer_out, \"abc\", 3);\r\n  evhtp_send_reply(req, EVHTP_RES_OK);\r\n  evhtp_request_resume(req);\r\n  \r\n  evtimer_del(e);\r\n  event_free(e);\r\n}\r\n\r\nvoid\r\ntestcb(evhtp_request_t * req, void * ptr)\r\n{\r\n  evhtp_request_pause(req);\r\n  \r\n  e = evtimer_new(evthr_get_base(evhtp_request_get_connection(req)->thread),\r\n                  eventcb, req);\r\n  \r\n  struct timeval tv = {1, 0};\r\n  evtimer_add(e, &tv);\r\n}\r\n\r\nint\r\nmain(int argc, char ** argv)\r\n{\r\n  evbase_t * evbase = event_base_new();\r\n  evhtp_t  * htp    = evhtp_new(evbase, NULL);\r\n  \r\n  evhtp_set_max_keepalive_requests(htp, 1);\r\n  \r\n  evhtp_set_gencb(htp, testcb, NULL);\r\n  evhtp_use_threads(htp, NULL, 4, NULL);\r\n  evhtp_bind_socket(htp, \"0.0.0.0\", 8081, 1024);\r\n\r\n  event_base_loop(evbase, 0);\r\n\r\n  evhtp_unbind_socket(htp);\r\n  evhtp_free(htp);\r\n  event_base_free(evbase);\r\n\r\n  return 0;\r\n}\r\n```\r\n\r\nI have traced the source code, and find the problem was caused by the `c->waiting` flag\r\nfile:`evhtp.c` function:`_evhtp_connection_resumecb`\r\n```c\r\n    /* XXX this is a hack to show a potential fix for issues/86, the main indea\r\n     * is that you call resume AFTER you have sent the reply (not BEFORE).\r\n     *\r\n     * When it has been decided this is a proper fix, the pause bit should be\r\n     * changed to a state-type flag.\r\n     */\r\n\r\n    if (evbuffer_get_length(bufferevent_get_output(c->bev))) {\r\n        bufferevent_enable(c->bev, EV_WRITE);\r\n        c->waiting = 1; // <---- HERE\r\n    } else {\r\n        bufferevent_enable(c->bev, EV_READ | EV_WRITE);\r\n        _evhtp_connection_readcb(c->bev, c);\r\n    }\r\n```\r\nAfter request resumed, this flag was set to 1, and `writecb` will return before the `max_keepalive_request` check\r\nfile:`evhtp.c` function:`_evhtp_connection_writecb`\r\n```c\r\n    if (evhtp_unlikely(c->waiting == 1)) {\r\n        c->waiting = 0;\r\n\r\n        bufferevent_enable(bev, EV_READ);\r\n\r\n        if (evbuffer_get_length(bufferevent_get_input(bev))) {\r\n            _evhtp_connection_readcb(bev, arg);\r\n        }\r\n\r\n        return; // <---- HERE\r\n    }\r\n\r\n    if (c->request->finished == 0 || evbuffer_get_length(bufferevent_get_output(bev))) {\r\n        return;\r\n    }\r\n\r\n    // CODE WHERE NEVER REACH HERE\r\n    /*\r\n     * if there is a set maximum number of keepalive requests configured, check\r\n     * to make sure we are not over it. If we have gone over the max we set the\r\n     * keepalive bit to 0, thus closing the connection.\r\n     */\r\n    if (c->htp->max_keepalive_requests) {\r\n        if (++c->num_requests >= c->htp->max_keepalive_requests) {\r\n            c->request->keepalive = 0;\r\n        }\r\n    }\r\n```"},"comment":{"url":"https://api.github.com/repos/ellzey/libevhtp/issues/comments/138804970","html_url":"https://github.com/ellzey/libevhtp/issues/184#issuecomment-138804970","issue_url":"https://api.github.com/repos/ellzey/libevhtp/issues/184","id":138804970,"user":{"login":"cuber","id":1525381,"avatar_url":"https://avatars.githubusercontent.com/u/1525381?v=3","gravatar_id":"","url":"https://api.github.com/users/cuber","html_url":"https://github.com/cuber","followers_url":"https://api.github.com/users/cuber/followers","following_url":"https://api.github.com/users/cuber/following{/other_user}","gists_url":"https://api.github.com/users/cuber/gists{/gist_id}","starred_url":"https://api.github.com/users/cuber/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cuber/subscriptions","organizations_url":"https://api.github.com/users/cuber/orgs","repos_url":"https://api.github.com/users/cuber/repos","events_url":"https://api.github.com/users/cuber/events{/privacy}","received_events_url":"https://api.github.com/users/cuber/received_events","type":"User","site_admin":false},"created_at":"2015-09-09T06:38:22Z","updated_at":"2015-09-09T06:38:22Z","body":"If the `c->waiting` flag is 1, the `_evhtp_connection_writecb` should not return while the input buffer is not empty\r\n```c\r\n    if (evhtp_unlikely(c->waiting == 1)) {\r\n        c->waiting = 0;\r\n\r\n        bufferevent_enable(bev, EV_READ);\r\n\r\n        if (evbuffer_get_length(bufferevent_get_input(bev))) {\r\n            return _evhtp_connection_readcb(bev, arg);\r\n        }\r\n    }\r\n```"}}