allen8807 / memcached

Automatically exported from code.google.com/p/memcached
0 stars 0 forks source link

code logic bug in drive_machine() #261

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
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;

Original issue reported on code.google.com by auxten...@gmail.com on 18 Mar 2012 at 6:45

GoogleCodeExporter commented 9 years ago
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.

Original comment by dorma...@rydia.net on 8 Dec 2013 at 8:52