[Libwebsockets] #331 client application crashes libwebsocket_context_destroy on flaky network tests

Vikas Gahlan vikas.gahlan at chargepoint.com
Mon Oct 19 17:42:23 CEST 2015


Initially, I also thought about same fix what you proposed but when I looked into lws_change_pollfd() function code, there are more error handling in this function where full cleanup using libwebsocket_close_and_free_session() not required IMO. As I am new to libwebsocket and still reviewing code to understand architecture , so I put changes in oom:  handling to ensure libwebsocket_close_and_free_session() is invoked only when there is valid entry in fdtable.

I am using multithreaded application in which libwebsocket_client_connect_2 is invoked in 2 different context at context setup: 
1) When my client application invokes libwebsocket_client_connect() after successful context creation using libwebsocket_create_context().
2) If libwebsocket_client_connect() is successful in #A which is mostly successful then app creates specific service thread for handling websocket communication.

If you are sure that lws_change_pollfd() is invoked only when sock is added in fdlist, then your fix of changes oom to failed in case of lws_change_pollfd() error is fine. 

Vikas Gahlan

-----Original Message-----
From: Andy Green [mailto:andy.green at linaro.org] On Behalf Of Andy Green
Sent: Sunday, October 18, 2015 4:25 PM
To: Vikas Gahlan; libwebsockets at ml.libwebsockets.org
Subject: Re: [Libwebsockets] #331 client application crashes in libwebsocket_context_destroy on flaky network tests



On 18 October 2015 18:39:39 GMT+08:00, Andy Green <andy at warmcat.com> wrote:
>
>
>On 18 October 2015 14:19:20 GMT+08:00, Vikas Gahlan 
><vikas.gahlan at chargepoint.com> wrote:
>>Attached patch work for me.
>>
>>Here is exact trace flow of the crash:
>>
>>libwebsocket_client_connect_2
>>libwebsocket_client_connect_2: address mywebsocket.com closed 
>>libwebsocket_context_destroy
>>close: just_kill_connection
>>remove_wsi_socket_from_fds: wsi=0x7a0d8, sock=9, fds pos=1
>>
>>
>>below fix works for me:
>>
>>From 300e50b2fe6a033a109d5f1f15208cb8490363e4 Mon Sep 17 00:00:00 2001
>>From: vgahlan-cpi <vikas.gahlan at chargepoint.com>
>>Date: Sat, 17 Oct 2015 22:40:23 -0700
>>Subject: [PATCH] fdpolllist cleanup requires if wsi socket exists in 
>>fdtable to avoid double free in libwebsocket_context_destroy
>>
>>---
>>lib/client-handshake.c | 9 ++++++---
>>1 file changed, 6 insertions(+), 3 deletions(-)
>>
>>diff --git a/lib/client-handshake.c b/lib/client-handshake.c index 
>>7293a41..d7d72e6 100644
>>--- a/lib/client-handshake.c
>>+++ b/lib/client-handshake.c
>>@@ -291,9 +291,12 @@ struct libwebsocket 
>>*libwebsocket_client_connect_2(
>>        return wsi;
>>
>>oom4:
>>-       lws_free(wsi->u.hdr.ah);
>>-       lws_free(wsi);
>>-       return NULL;
>>+        /*In case wsi added successfully in fd polllist, clean-up
>>requires for fdtable and session data properly using 
>>libwebsocket_close_and_free_session().*/
>>+        if(wsi->position_in_fds_table < 0  || wsi->sock < 0 ){
>
>Thanks... this definitely is a real problem, but the fix needs to be a 
>bit closer to the source of the problem I think.
>
>I'll push an alternative fix for you to try in the next minutes.

How about this?  He was the only path to oom4 after insertion into the fds list.

http://git.libwebsockets.org/cgi-bin/cgit/libwebsockets/commit/?id=b5cf69fdb57c4025572cde1007e0ff16108d4f80

-Andy

>-Andy
>
>>+                lws_free(wsi->u.hdr.ah);
>>+                lws_free(wsi);
>>+                return NULL;
>>+        }
>>
>>failed:
>>        libwebsocket_close_and_free_session(context, wsi,
>>--
>>1.8.3.1
>>
>>Vikas Gahlan
>>ChargePoint | chargepoint.com
>>Ofiice: +91-124-4889450 Ext. 464 | Mobile +91-9873232008
>>
>>
>>
>>
>>
>>
>>
>>
>>This email and any attachments are intended for the sole use of the 
>>intended recipient(s) and
>>contain(s) confidential information that may be proprietary,
>privileged
>>or copyrighted under
>>applicable law. If you are not the intended recipient, do not read, 
>>copy, or forward this email message or any attachments and delete this 
>>email message and any attachments immediately.
>>
>>----------------------------------------------------------------------
>>--
>>
>>_______________________________________________
>>Libwebsockets mailing list
>>Libwebsockets at ml.libwebsockets.org
>>http://ml.libwebsockets.org/mailman/listinfo/libwebsockets
>
>_______________________________________________
>Libwebsockets mailing list
>Libwebsockets at ml.libwebsockets.org
>http://ml.libwebsockets.org/mailman/listinfo/libwebsockets







This email and any attachments are intended for the sole use of the intended recipient(s) and
 contain(s) confidential information that may be proprietary, privileged or copyrighted under
 applicable law. If you are not the intended recipient, do not read, copy, or forward this email
 message or any attachments and delete this email message and any attachments immediately.


More information about the Libwebsockets mailing list