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

Yunkai Zhang yunkai.me at gmail.com
Thu Jul 5 20:16:40 CEST 2012



在 2012-7-6,1:42,MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp> 写道:

> 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.

Ho, you are right! I'll give v2 tomorrow.


> 
> Thanks,
> 
> Kazutaka



More information about the sheepdog mailing list