<p></p>
<p dir="auto">Hi Daniel,</p>
<p dir="auto">Thank you for the prompt response. We are looking into migrating to 5.5.x shortly.</p>
<p dir="auto">We did add some additional debugging and got an interesting result.</p>
<pre><code>Dec 31 18:29:08.819765 eqx-sjc-ecv3 kamailio_edge[19096]: WARNING: websocket [ws_frame.c:810]: ws_keepalive(): forcibly closing connection
> grep "tcp id = 19132" /var/log/kamailio/kamailio_edge.log
Dec 31 18:29:07.815876 eqx-sjc-ecv3 kamailio_edge[19096]: INFO: websocket [ws_conn.c:371]: wsconn_close_now(): wsconn_close_now: wsc = 0xa2ac94e0, tcp con = 0x9edb0e80, tcp id = 19132
Dec 31 18:29:08.819965 eqx-sjc-ecv3 kamailio_edge[19096]: INFO: websocket [ws_conn.c:371]: wsconn_close_now(): wsconn_close_now: wsc = 0xa2ac94e0, tcp con = 0x00000000, tcp id = 19132
Dec 31 18:29:09.824694 eqx-sjc-ecv3 kamailio_edge[19096]: INFO: websocket [ws_conn.c:371]: wsconn_close_now(): wsconn_close_now: wsc = 0xa2ac94e0, tcp con = 0x00000000, tcp id = 19132
Dec 31 18:29:10.833513 eqx-sjc-ecv3 kamailio_edge[19096]: INFO: websocket [ws_conn.c:371]: wsconn_close_now(): wsconn_close_now: wsc = 0xa2ac94e0, tcp con = 0x00000000, tcp id = 19132
Dec 31 18:29:11.838090 eqx-sjc-ecv3 kamailio_edge[19096]: INFO: websocket [ws_conn.c:371]: wsconn_close_now(): wsconn_close_now: wsc = 0xa2ac94e0, tcp con = 0x00000000, tcp id = 19132
Dec 31 18:29:12.841246 eqx-sjc-ecv3 kamailio_edge[19096]: INFO: websocket [ws_conn.c:371]: wsconn_close_now(): wsconn_close_now: wsc = 0xa2ac94e0, tcp con = 0x00000000, tcp id = 19132
Dec 31 18:38:20.299126 eqx-sjc-ecv3 kamailio_edge[19218]: INFO: <core> [core/tcp_main.c:1584]: _tcpconn_detach(): _tcpconn_detach: tcpconn=0x9edb0e80, tcp id = 19132
</code></pre>
<p dir="auto">I did want to get your thoughts on the current implementation of the <code>ws_keepalive</code> functionality.</p>
<pre><code> if(wsc->state == WS_S_CLOSING || wsc->awaiting_pong) {
LM_WARN("forcibly closing connection\n");
wsconn_close_now(wsc);
</code></pre>
<p dir="auto">When <code>wsconn_close_now</code> is called, the connection state is set to <code>S_CONN_BAD</code>.<br>
On subsequent <code>wsconn_close_now</code> calls, <code>tcpconn_get</code> -> <code>_tcpconn_find</code> calls will return 0 due to <a href="https://github.com/dialpad/kamailio/blob/master/src/core/tcp_main.c#L1650">this line</a>: <code>if ((id==c->id)&&(c->state!=S_CONN_BAD)) {</code>.</p>
<p dir="auto">Ie. <code>_tcpconn_find</code> skips over connections in the <code>S_CONN_BAD</code> state.</p>
<p dir="auto">Why is <code>wsconn_close_now</code> being called multiple times? It looks like <code>ws_keepalive</code> isn't removing the websocket connection from the list of open connections. Instead, it's <a href="https://github.com/kamailio/kamailio/blob/master/src/modules/websocket/ws_frame.c#L810">de-referencing an extra time</a>.</p>
<pre><code>void ws_keepalive(unsigned int ticks, void *param)
{
...
while(list_head[i].id!=-1) {
//For each websocket connection, add a refcount
wsc = wsconn_get(list_head[i].id);
if(wsc && wsc->last_used < check_time) {
if(wsc->state == WS_S_CLOSING || wsc->awaiting_pong) {
LM_WARN("forcibly closing connection\n");
//Remove extra refcount?
wsconn_close_now(wsc);
} else if (ws_keepalive_mechanism == KEEPALIVE_MECHANISM_CONCHECK) {
tcp_connection_t *con = tcpconn_get(wsc->id, 0, 0, 0, 0);
if(con==NULL) {
LM_INFO("tcp connection has been lost\n");
wsc->state = WS_S_CLOSING;
} else {
tcpconn_put(con);
}
} else {
int opcode = (ws_keepalive_mechanism == KEEPALIVE_MECHANISM_PING)
? OPCODE_PING
: OPCODE_PONG;
ping_pong(wsc, opcode);
}
}
if(wsc) {
//Remove refcount
wsconn_put_id(list_head[i].id);
}
i++;
}
wsconn_put_list_ids(list_head);
}
</code></pre>
<p dir="auto">What about modifying the keepalive to do something like this instead?</p>
<pre><code>for(i = 0; list_head[i].id!=-1; i++) {
wsc = wsconn_get(list_head[i].id);
if(wsc->state == WS_S_CLOSING) {
/* Skip closing connections */
continue;
} else if (wsc->awaiting_pong) {
LM_WARN("forcibly closing connection, no pong received\n");
wsconn_close_now(wsc);
continue;
} else if ...
</code></pre>
<p dir="auto">On skipping <code>wsconn_close_now</code> in the <code>WS_S_CLOSING</code> case, should it really be the responsibility of the keepalive to close the connection in that case?</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/kamailio/kamailio/issues/2990#issuecomment-1006088547">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABO7UZIYTJJ726KP2BTFFQLUUSZHXANCNFSM5K6ZG4QA">unsubscribe</a>.<br />Triage notifications on the go with GitHub Mobile for <a href="https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675">iOS</a> or <a href="https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub">Android</a>.
<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/ABO7UZP6O52VD6KXAZZC35LUUSZHXA5CNFSM5K6ZG4QKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHP33CYY.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><kamailio/kamailio/issues/2990/1006088547</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/kamailio/kamailio/issues/2990#issuecomment-1006088547",
"url": "https://github.com/kamailio/kamailio/issues/2990#issuecomment-1006088547",
"name": "View Issue"
},
"description": "View this Issue on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>