#788 crash in ConnectionExists

closed-fixed
libcurl (355)
7
2013-06-21
2008-11-26
Igor
No

The connection is destroyed, but it is not removed form the connection cache. The crash occurs when this freed connection is found in the cache and is tried to be reused.

Full log is attached.
Possible patch is attached.

Windows XP 32 bit, curl-7.19.3-20081126

Crash in ConnectionExists() in line:
pipeLen = check->send_pipe->size + check->recv_pipe->size;
check=data->state.connc->connects[i=0]=0x003f2780 was freed

backtrace
---------
ConnectionExists(SessionHandle * 0x00e7c028, connectdata * 0x00e6a4e0, connectdata * * 0x0012f8f4) line 2461 + 18 bytes
create_conn(SessionHandle * 0x00e7c028, connectdata * * 0x00e6ab1c, Curl_dns_entry * * 0x0012fb24, unsigned char * 0x0012fb98) line 4318 + 23 bytes
Curl_connect(SessionHandle * 0x00e7c028, connectdata * * 0x00e6ab1c, unsigned char * 0x0012fb98, unsigned char * 0x0012fb9c) line 4512 + 21 bytes
multi_runsingle(Curl_multi * 0x003fe9e8, Curl_one_easy * 0x00e6ab10) line 981 + 27 bytes
multi_socket(Curl_multi * 0x003fe9e8, unsigned char 0, unsigned int 4294967295, int 0, int * 0x0012fc60) line 1874 + 19 bytes
curl_multi_socket_action(void * 0x003fe9e8, unsigned int 4294967295, int 0, int * 0x0012fc60) line 1964 + 23 bytes
my timer expired callback: CurlTimerCB(void * 0x003fbb20, RvTimer * 0x0012fcd8) line 1966 + 24 bytes

Log snip
--------
CurlCB_debug(request=00E7CDF0,easy=00E7CE88): Connected to xcap.radvision.com (172.28.2.176) port 80 (#0,0x3f2780)
CurlCB_debug(request=00E6A110,easy=00E85470): curl_multi_add_handle: easy=0xe6a7a8->easy_handle=0xe85470
CurlCB_debug(request=00E6A110,easy=00E85470): create_conn: conn=0xe66308,connectindex=-1
CurlCB_debug(request=00E6A110,easy=00E85470): ConnectionExists: check/conn=0x3f2780
CurlCB_debug(request=00E6A110,easy=00E85470): create_conn: conn=0xe66308,conn_temp=0x3f2780(connectindex=0)
CurlCB_debug(request=00E6A110,easy=00E85470): create_conn: conn=0xe66308 was freed
CurlCB_debug(request=00E6A110,easy=00E85470): Re-using existing connection! (#0,0x3f2780) with host xcap.radvision.com
CurlCB_debug(request=00E7CDF0,easy=00E7CE88): curl_multi_remove_handle: curl_handle=0xe7ce88
CurlCB_debug(request=00E7CDF0,easy=00E7CE88): curl_multi_remove_handle: easy=0x3f2b68->easy_handle=0xe7ce88,conn=0x3f2780,connectindex=-1
CurlCB_debug(request=00E6A110,easy=00E85470): curl_multi_remove_handle: curl_handle=0xe85470
CurlCB_debug(request=00E6A110,easy=00E85470): Curl_done: conn=0x3f2780, connectindex=-1
CurlCB_debug(request=00E6A110,easy=00E85470): Curl_disconnect: conn=0x3f2780(connectindex=-1) was freed
CurlCB_debug(request=00E6A110,easy=00E85470): curl_multi_remove_handle: easy=0xe6a7a8->easy_handle=0xe85470,conn=(nil),connectindex=-1
CurlCB_debug(request=003F2970,easy=00E7C028): curl_multi_add_handle: easy=0xe6ab10->easy_handle=0xe7c028
CurlCB_debug(request=003F2970,easy=00E7C028): create_conn: conn=0xe6a4e0,connectindex=-1
CurlCB_debug(request=003F2970,easy=00E7C028): ConnectionExists: check/conn=0x3f2780

Discussion

1 2 > >> (Page 1 of 2)
  • Igor
    Igor
    2008-11-26

    full log + possible patch

     
  • I'm a bit curious on how you end up in this situation.

    Are you using pipelining and multiple easy handles? Is this (fix) happening when the last of those are removed from the multi handle?

     
    • priority: 5 --> 7
     
  • Igor
    Igor
    2008-11-30

    I do use pipeline, one multi object and multiple easy objects, as it can be seen from the attached early log. My scenario generates 4 simultaneous requests towards 2 servers (2 requests for one server and 2 - for other), establishing thus 2 persistent connections. The request sequence is infinite. Usually It take ~100 requests before scenario crashes.

    Few notes about crash:
    1. May be it is imortant, the crash appears in the version, patched with the fix for bug #2351645, that I made in order to continue testing. The fix was proposed by me while reporting the bug.
    2. The scenario reproduces crash every run, but sometimes the crash occurs in other places. In all cases the crash is caused by the access of connection found in cache,when the connection memory was freed.

     
  • I think the problem with this patch (apart from the irrelevant changes to infof calls) is that it clears stuff in the connection cache, while removing an easy handle from the multi handle shouldn't have to touch that, and in fact I don't think it should even be allowed.

    What data that is associated with the *connection* is freed when this easy handle is removed?

     
  • Igor
    Igor
    2008-12-03

    screenshot of crashed code, MSDEV 6.0

     
    Attachments
  • Igor
    Igor
    2008-12-03

    Which easy do you mean by "this easy handle is removed"?

    The whole connection object memory is freed.
    Please find the attached screenshot of the debugger at the moment of the crash.

    File Added: Screenshot.jpg

     
  • An easy handle added to a multi handle is basically a 'struct SessionHandle'. Such a struct can use a connection from the connection cache, but it doesn't _own_ the connection. When the easy handle is removed from the multi handle the connection is supposed to be returned to the cache and not meddled with (in normal cases). So in this case, why is the connection freed? And if it is freed, why isn't it properly removed from the connection cache by the code that frees it?

    I don't doubt what you see in the debugger, I'm just saying that the suggested patch has a logical flaw even if it happens to cure the problem for you.

    I assume you can't provide a test app that reproduces this case very easily?

     
  • Igor
    Igor
    2008-12-09

    The connection was freed by the following flow:
    curl_multi_remove_handle -> Curl_done -> Curl_disconnect -> conn_free
    The connection was not removed from the cache by Curl_disconnect,
    because conn->connectindex was reset to -1 from within curl_multi_remove_handle called for other handle just before this flow took a place.
    Note this previous call to curl_multi_remove_handle didn't call Curl_done, because (easy->easy_conn->data == easy->easy_handle) check failed.

    In ligth of the told above,
    I propose to fix the problem in the other way:
    instead of removal connection from the cache simply prevent reset of conn->connectindex to -1. I attached the patch.
    What do you think of it?

    File Added: fix_crash_in_ConnectionExists_2.diff

     
  • Igor
    Igor
    2008-12-09

    the latest version of patch

     
1 2 > >> (Page 1 of 2)