Export to GitHub

memcached - issue #261

code logic bug in drive_machine()


Posted on Mar 18, 2012 by Swift Cat

3839 case conn_new_cmd: 3840 /* Only process nreqs at a time to avoid starving other 3841 connections / 3842 3843 --nreqs; 3844 if (nreqs >= 0) { 3845 reset_cmd_handler(c); 3846 } else { 3847 pthread_mutex_lock(&c->thread->stats.mutex); 3848 c->thread->stats.conn_yields++; 3849 pthread_mutex_unlock(&c->thread->stats.mutex); 3850 if (c->rbytes > 0) { 3851 / We have already read in data into the input buffer, 3852 so libevent will most likely not signal read events 3853 on the socket (unless more data is available. As a 3854 hack we should just put in a request to write data, 3855 because that should be possible ;-) 3856 */ 3857 if (!update_event(c, EV_WRITE | EV_PERSIST)) { 3858 if (settings.verbose > 0) 3859 fprintf(stderr, "Couldn't update event\n"); 3860 conn_set_state(c, conn_closing); 3861 } 3862 } 3863 stop = true; 3864 } 3865 break;

on line 3857 if update_event() return false and the connection's bind event is still EV_READ c->state will be conn_closing and stop will be true if no more bytes comes, the socket will never be cleanup?

so i think there need a patch

--- a/memcached.c +++ b/memcached.c @@ -3858,6 +3858,7 @@ static void drive_machine(conn *c) { if (settings.verbose > 0) fprintf(stderr, "Couldn't update event\n"); conn_set_state(c, conn_closing); + break; } } stop = true;

Comment #1

Posted on Dec 8, 2013 by Helpful Bird

Wow, two years old... and it looks correct to me. If that update_event fails the connection might zombie. It's very hard for that to fail and it's been that way forever.

Pushed a commit for the next release.

Status: Fixed

Labels:
Type-Defect Priority-Medium