[sr-dev] git:master: modules/websocket: Fix connection leaks

Hugh Waite hugh.waite at crocodile-rcs.com
Fri Jul 5 15:24:15 CEST 2013


Hello,
I found that the websocket module was not using the tcp connections 
properly causing the tcpconn structure, including buffer, to remain in 
memory due to a positive reference count. (These are hard to track 
because they are no longer in the core.tcp_list output!)
This commit tried to fix that by running tcpconn_put after each 
tcpconn_get. Please speak up if I'm not using the tcpconn put/get 
mechanism correctly!
Most of the time, a websocket connection that is set up and closed is 
fine and releases the connection memory. However I am still getting some 
leaks of tcpconn structures. I assume this is still due to a positive 
refcount somehow.

Can someone help point me in the right direction with this?
Should I be using tcpconn_chld_put in some places, or checking the 
result of tcpconn_put?

Thanks,
Hugh

On 04/07/2013 10:34, Hugh Waite wrote:
> Module: sip-router
> Branch: master
> Commit: 27474179bdeef0ddaba05389f510446a387d85e1
> URL:    http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=27474179bdeef0ddaba05389f510446a387d85e1
>
> Author: Hugh Waite <hugh.waite at crocodile-rcs.com>
> Committer: Hugh Waite <hugh.waite at crocodile-rcs.com>
> Date:   Thu Jul  4 10:31:46 2013 +0100
>
> modules/websocket: Fix connection leaks
>
> - Decrease the TCP connection reference count after each use
>
> ---
>
>   modules/websocket/ws_conn.c      |    5 +++++
>   modules/websocket/ws_frame.c     |    5 +++++
>   modules/websocket/ws_handshake.c |   23 ++++++++++++++---------
>   3 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/modules/websocket/ws_conn.c b/modules/websocket/ws_conn.c
> index aeda6b2..27a9968 100644
> --- a/modules/websocket/ws_conn.c
> +++ b/modules/websocket/ws_conn.c
> @@ -356,6 +356,7 @@ void wsconn_close_now(ws_connection_t *wsc)
>   		return;
>   	}
>   
> +	tcpconn_put(con);
>   	con->send_flags.f |= SND_F_CON_CLOSE;
>   	con->state = S_CONN_BAD;
>   	con->timeout = get_ticks_raw();
> @@ -422,8 +423,12 @@ static int add_node(struct mi_root *tree, ws_connection_t *wsc)
>   					pong,
>   					interval,
>   					sub_protocol) == 0)
> +		{
> +			tcpconn_put(con);
>   			return -1;
> +		}
>   
> +		tcpconn_put(con);
>   		return 1;
>   	}
>   	else
> diff --git a/modules/websocket/ws_frame.c b/modules/websocket/ws_frame.c
> index 263f4d2..086cf79 100644
> --- a/modules/websocket/ws_frame.c
> +++ b/modules/websocket/ws_frame.c
> @@ -240,6 +240,7 @@ static int encode_and_send_ws_frame(ws_frame_t *frame, conn_close_t conn_close)
>   		if (wsconn_rm(frame->wsc, WSCONN_EVENTROUTE_YES) < 0)
>   		{
>   			LM_ERR("removing WebSocket connection\n");
> +			tcpconn_put(con);
>   			pkg_free(send_buf);
>   			return -1;
>   		}
> @@ -252,6 +253,7 @@ static int encode_and_send_ws_frame(ws_frame_t *frame, conn_close_t conn_close)
>   			STATS_TX_DROPS;
>   			LM_WARN("TCP disabled\n");
>   			pkg_free(send_buf);
> +			tcpconn_put(con);
>   			return -1;
>   		}		
>   	}
> @@ -263,6 +265,7 @@ static int encode_and_send_ws_frame(ws_frame_t *frame, conn_close_t conn_close)
>   			STATS_TX_DROPS;
>   			LM_WARN("TLS disabled\n");
>   			pkg_free(send_buf);
> +			tcpconn_put(con);
>   			return -1;
>   		}		
>   	}
> @@ -293,6 +296,7 @@ static int encode_and_send_ws_frame(ws_frame_t *frame, conn_close_t conn_close)
>   			update_stat(ws_msrp_failed_connections, 1);
>   		if (wsconn_rm(frame->wsc, WSCONN_EVENTROUTE_YES) < 0)
>   			LM_ERR("removing WebSocket connection\n");
> +		tcpconn_put(con);
>   		return -1;
>   	}
>   
> @@ -308,6 +312,7 @@ static int encode_and_send_ws_frame(ws_frame_t *frame, conn_close_t conn_close)
>   	}
>   	
>   	pkg_free(send_buf);
> +	tcpconn_put(con);
>   	return 0;
>   }
>   
> diff --git a/modules/websocket/ws_handshake.c b/modules/websocket/ws_handshake.c
> index a719919..1ca6fe1 100644
> --- a/modules/websocket/ws_handshake.c
> +++ b/modules/websocket/ws_handshake.c
> @@ -154,7 +154,7 @@ int ws_handle_handshake(struct sip_msg *msg)
>   	if (con->type != PROTO_TCP && con->type != PROTO_TLS)
>   	{
>   		LM_ERR("unsupported transport: %d", con->type);
> -		return 0;
> +		goto end;
>   	}
>   
>   	if (parse_headers(msg, HDR_EOH_F, 0) < 0)
> @@ -162,7 +162,7 @@ int ws_handle_handshake(struct sip_msg *msg)
>   		LM_ERR("error parsing headers\n");
>   		ws_send_reply(msg, 500, &str_status_internal_server_error,
>   				NULL);
> -		return 0;
> +		goto end;
>   	}
>   
>   	/* Process HTTP headers */
> @@ -209,7 +209,7 @@ int ws_handle_handshake(struct sip_msg *msg)
>   				ws_send_reply(msg, 400,
>   						&str_status_bad_request,
>   						NULL);
> -				return 0;
> +				goto end;
>   			}
>   
>   			LM_DBG("found %.*s: %.*s\n",
> @@ -253,7 +253,7 @@ int ws_handle_handshake(struct sip_msg *msg)
>   				ws_send_reply(msg, 400,
>   						&str_status_bad_request,
>   						NULL);
> -				return 0;
> +				goto end;
>   			}
>   
>   			str2sint(&hdr->body, &version);
> @@ -271,7 +271,7 @@ int ws_handle_handshake(struct sip_msg *msg)
>   				ws_send_reply(msg, 426,
>   						&str_status_upgrade_required,
>   						&headers);
> -				return 0;
> +				goto end;
>   			}
>   
>   			LM_DBG("found %.*s: %.*s\n",
> @@ -291,7 +291,7 @@ int ws_handle_handshake(struct sip_msg *msg)
>   				ws_send_reply(msg, 400,
>   						&str_status_bad_request,
>   						NULL);
> -				return 0;
> +				goto end;
>   			}
>   
>   			LM_DBG("found %.*s: %.*s\n",
> @@ -337,7 +337,7 @@ int ws_handle_handshake(struct sip_msg *msg)
>   					str_hdr_sec_websocket_version.s,
>   					WS_VERSION);
>   		ws_send_reply(msg, 400, &str_status_bad_request, &headers);
> -		return 0;
> +		goto end;
>   	}
>   
>   	/* Construct reply_key */
> @@ -348,7 +348,7 @@ int ws_handle_handshake(struct sip_msg *msg)
>   		LM_ERR("allocating pkg memory\n");
>   		ws_send_reply(msg, 500, &str_status_internal_server_error,
>   				NULL);
> -		return 0;
> +		goto end;
>   	}
>   	memcpy(reply_key.s, key.s, key.len);
>   	memcpy(reply_key.s + key.len, str_ws_guid.s, str_ws_guid.len);
> @@ -424,7 +424,7 @@ int ws_handle_handshake(struct sip_msg *msg)
>   		if ((wsc = wsconn_get(msg->rcv.proto_reserved1)) != NULL)
>   			wsconn_rm(wsc, WSCONN_EVENTROUTE_NO);
>   
> -		return 0;
> +		goto end;
>   	}
>   	else
>   	{
> @@ -434,7 +434,12 @@ int ws_handle_handshake(struct sip_msg *msg)
>   			update_stat(ws_msrp_successful_handshakes, 1);
>   	}
>   
> +	tcpconn_put(con);
>   	return 1;
> +end:
> +	if (con)
> +		tcpconn_put(con);
> +	return 0;
>   }
>   
>   struct mi_root *ws_mi_disable(struct mi_root *cmd, void *param)
>
>
> _______________________________________________
> sr-dev mailing list
> sr-dev at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev


-- 
Hugh Waite
Principal Design Engineer
Crocodile RCS Ltd.




More information about the sr-dev mailing list