[sheepdog] [PATCH 1/3] sheep: add helper function and clear connection directly when it is dead

[MORITA Kazutaka]

At Fri, 6 Jul 2012 01:33:44 +0800,
Yunkai Zhang wrote:
> 
> On Fri, Jul 6, 2012 at 12:57 AM, MORITA Kazutaka
> <morita.kazutaka at lab.ntt.co.jp> wrote:
> > At Thu,  5 Jul 2012 18:34:10 +0800,
> > Yunkai Zhang wrote:
> >>
> >> From: Yunkai Zhang <qiushu.zyk at taobao.com>
> >>
> >> When connection is dead, we should:
> >>  1) do some clear actions about this connection.
> >>  2) unregister it's fd from epoll.
> >>  3) destroy this connection directly, since it's fd has been unregistered,
> >>     sheep couldn't receive any new events of this fd.
> >>     *Note*: If we use client_decref() instread of destroy_client(), this fd
> >>     may not be destroyed when ci->refcnt is larger than 1, than this fd could
> >>     not be closed in the future.
> >>
> >> Now I add a helper named clear_client() to do these works.
> >>
> >> Signed-off-by: Yunkai Zhang <qiushu.zyk at taobao.com>
> >> ---
> >>  sheep/sdnet.c |   20 ++++++++++++--------
> >>  1 file changed, 12 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/sheep/sdnet.c b/sheep/sdnet.c
> >> index c13cdb0..b493428 100644
> >> --- a/sheep/sdnet.c
> >> +++ b/sheep/sdnet.c
> >> @@ -370,6 +370,7 @@ static void requeue_request(struct request *req)
> >>
> >>  static void client_incref(struct client_info *ci);
> >>  static void client_decref(struct client_info *ci);
> >> +static void clear_client(struct client_info *ci);
> >>
> >>  static struct request *alloc_local_request(void *data, int data_length)
> >>  {
> >> @@ -473,11 +474,10 @@ void put_request(struct request *req)
> >>               if (conn_tx_on(&ci->conn)) {
> >>                       dprintf("connection seems to be dead\n");
> >>                       free_request(req);
> >> +                     clear_client(ci);
> >
> > We cannot free the client_info here because there may be other
> > inflight requests which use it, no?
> 
> 
> Yes, there may be other inflight request, but since conn_tx_on()
> failed and then we can't send back response to the client, why not
> free it? I think it can make program simple.

If we call put_request against the inflight requests after freeing the
client_info, conn_tx_on(&ci->conn) can lead to segfaults because ci is
no longer a valid value.

Thanks,

Kazutaka