[sr-dev] Memory leaks in websocket module

[Daniel-Constantin Mierla]

Hello,

can you test with kamailio 5.2.2, it should have all the patches needed
after the last changes.

If pkg_malloc failed, you can increase the value that can be used by
kamailio with -M command line parameter. We can add an option to use shm
or even the standard malloc in this case.

However, your changes seem to revert code from last changes that were
solving a race, so the way to go would be to add the option for
connection check instead of ping/pong to the existing code. That could
be also sort of default behaviour, because if tcp connection is gone,
then no ws connection structure should be kept. I need to go through the
code and see if it is not done already in a different part. Anyhow,
adding the tcp connection check option should be easy. I will add it
soon if nobody else makes a pull request meanwhile.

Cheers,
Daniel

On 15.03.19 13:23, Andrey Deykunov wrote:
> Hi guys,
>
> We had Kamailio 5.1.4 with websocket module. Unfortunately, our
> clients don't support websocket keepalive mechanism at all, so I used
> TCP keepalive instead with the following parameters:
>
> tcp_keepalive=yes
> tcp_keepcnt=6
> tcp_keepidle=60
> tcp_keepintvl=10
>
>
> and set up KEEPALIVE_MECHANISM_NONE:
>
> modparam("websocket", "keepalive_mechanism", 0)
>
>
> During load testing and debugging, when 8k clients sent registrations,
> it was found out that shared memory was not freed after closing
> connections. So I've decided to add new keepalive mechanism that
> periodically checks TCP connection related to websocket:
>
> enum
> {
>     KEEPALIVE_MECHANISM_NONE = 0,
>     KEEPALIVE_MECHANISM_PING = 1,
>     KEEPALIVE_MECHANISM_PONG = 2,
>     KEEPALIVE_MECHANISM_TCP_CONN_CHECK = 3
> };  
>
>
> and added the line to config:
>
> # Enable custom tcp-connection-health based keepalive mechanism (3)
> # KEEPALIVE_MECHANISM_NONE = 0,
> # KEEPALIVE_MECHANISM_PING = 1,
> # KEEPALIVE_MECHANISM_PONG = 2
> # KEEPALIVE_MECHANISM_TCP_CONN_CHECK = 3
> modparam("websocket", "keepalive_mechanism", 3)
>
>
> Also, I've implemented the mechanism in ws_keepalive function:
>
> void ws_keepalive(unsigned int ticks, void *param)
> {
>     int check_time =
>             (int)time(NULL) - cfg_get(websocket, ws_cfg,
> keepalive_timeout);
>     ws_connection_t **list = NULL, **list_head = NULL;
>     ws_connection_t *wsc = NULL;
>     /* get an array of pointer to all ws connection */
>     list_head = wsconn_get_list();
>     if(!list_head)
>         return;
>     list = list_head;
>     wsc = *list_head;
>     while(wsc && wsc->last_used < check_time) {
>         if (ws_keepalive_mechanism ==
> KEEPALIVE_MECHANISM_TCP_CONN_CHECK) {
>             struct tcp_connection *con = tcpconn_get(wsc->id, 0, 0, 0, 0);
>             if(!con) {
>                 LM_INFO("tcp connection has been lost\n");
>                 wsc->state = WS_S_CLOSING;
>             }
>         }
>         if(wsc->state == WS_S_CLOSING || wsc->awaiting_pong) {
>             LM_INFO("forcibly closing connection\n");
>             wsconn_close_now(wsc);
>         } else {
>             int opcode = (ws_keepalive_mechanism ==
> KEEPALIVE_MECHANISM_PING)
>                                 ? OPCODE_PING
>                                 : OPCODE_PONG;
>             ping_pong(wsc, opcode);
>         }
>         wsc = *(++list);
>     }
>     wsconn_put_list(list_head);
> }
>
>
> and changed memory allocation method in wsconn_get_list and
> wsconn_put_list methods from pkg to shm, because, as it turned out
> during load testing, 
> pkg_malloc (the C malloc) may cousing fails under huge loads.
>
> These modifications solved the problem. But about a week ago we've
> started switching to ver. 5.2.1 and found a lot of changes in the
> websocket module. So, I've added my changes in this
> commit https://github.com/korizza/kamailio/commit/b3e03d03574ff4ff076005bb8a01d7461af2f8f5 .
> Please take a look.
>
> BTW: adding ws_conn_put_id in this
> commit https://github.com/kamailio/kamailio/commit/a975bca1702ea2f3db47f834f7e4da2786ced201#diff-59c50f19ab1ccf4afe10617cdc346bc2 did
> not solve problem with ref counter increasing.
>
>
> Thanks,
> Andrey Deykunov
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> _______________________________________________
> Kamailio (SER) - Development Mailing List
> sr-dev at lists.kamailio.org
> https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

-- 
Daniel-Constantin Mierla -- www.asipto.com
www.twitter.com/miconda -- www.linkedin.com/in/miconda
Kamailio World Conference - May 6-8, 2019 -- www.kamailioworld.com
Kamailio Advanced Training - Mar 25-27, 2019, in Washington, DC, USA -- www.asipto.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.kamailio.org/pipermail/sr-dev/attachments/20190316/1080a325/attachment-0001.html>