Browse Source

Add a test for websocket hibernate + timeout and fix this use case

The issue was that we were calling erlang:hibernate before a
receive .. after .. end call. Erlang hibernates the process before
reaching the receive instruction and we therefore couldn't enter
the after clause when hibernating.

This is now fixed by using erlang:send_after instead and receiving
that message instead of using an after clause.
Loïc Hoguin 13 years ago
parent
commit
d0f711a61d
3 changed files with 80 additions and 7 deletions
  1. 20 5
      src/cowboy_http_websocket.erl
  2. 31 2
      test/http_SUITE.erl
  3. 29 0
      test/ws_timeout_hibernate_handler.erl

+ 20 - 5
src/cowboy_http_websocket.erl

@@ -52,6 +52,7 @@
 	opts :: any(),
 	opts :: any(),
 	challenge = undefined :: undefined | binary(),
 	challenge = undefined :: undefined | binary(),
 	timeout = infinity :: timeout(),
 	timeout = infinity :: timeout(),
+	timeout_ref = undefined :: undefined | reference(),
 	messages = undefined :: undefined | {atom(), atom(), atom()},
 	messages = undefined :: undefined | {atom(), atom(), atom()},
 	hibernate = false :: boolean(),
 	hibernate = false :: boolean(),
 	eop :: undefined | tuple(), %% hixie-76 specific.
 	eop :: undefined | tuple(), %% hixie-76 specific.
@@ -170,16 +171,28 @@ handler_before_loop(State=#state{hibernate=true},
 		Req=#http_req{socket=Socket, transport=Transport},
 		Req=#http_req{socket=Socket, transport=Transport},
 		HandlerState, SoFar) ->
 		HandlerState, SoFar) ->
 	Transport:setopts(Socket, [{active, once}]),
 	Transport:setopts(Socket, [{active, once}]),
-	erlang:hibernate(?MODULE, handler_loop, [State#state{hibernate=false},
+	State2 = handler_loop_timeout(State),
+	erlang:hibernate(?MODULE, handler_loop, [State2#state{hibernate=false},
 		Req, HandlerState, SoFar]);
 		Req, HandlerState, SoFar]);
 handler_before_loop(State, Req=#http_req{socket=Socket, transport=Transport},
 handler_before_loop(State, Req=#http_req{socket=Socket, transport=Transport},
 		HandlerState, SoFar) ->
 		HandlerState, SoFar) ->
 	Transport:setopts(Socket, [{active, once}]),
 	Transport:setopts(Socket, [{active, once}]),
-	handler_loop(State, Req, HandlerState, SoFar).
+	State2 = handler_loop_timeout(State),
+	handler_loop(State2, Req, HandlerState, SoFar).
+
+-spec handler_loop_timeout(#state{}) -> #state{}.
+handler_loop_timeout(State=#state{timeout=infinity}) ->
+	State#state{timeout_ref=undefined};
+handler_loop_timeout(State=#state{timeout=Timeout, timeout_ref=PrevRef}) ->
+	_ = case PrevRef of undefined -> ignore; PrevRef ->
+		erlang:cancel_timer(PrevRef) end,
+	TRef = make_ref(),
+	erlang:send_after(Timeout, self(), {?MODULE, timeout, TRef}),
+	State#state{timeout_ref=TRef}.
 
 
 %% @private
 %% @private
 -spec handler_loop(#state{}, #http_req{}, any(), binary()) -> ok.
 -spec handler_loop(#state{}, #http_req{}, any(), binary()) -> ok.
-handler_loop(State=#state{messages={OK, Closed, Error}, timeout=Timeout},
+handler_loop(State=#state{messages={OK, Closed, Error}, timeout_ref=TRef},
 		Req=#http_req{socket=Socket}, HandlerState, SoFar) ->
 		Req=#http_req{socket=Socket}, HandlerState, SoFar) ->
 	receive
 	receive
 		{OK, Socket, Data} ->
 		{OK, Socket, Data} ->
@@ -189,11 +202,13 @@ handler_loop(State=#state{messages={OK, Closed, Error}, timeout=Timeout},
 			handler_terminate(State, Req, HandlerState, {error, closed});
 			handler_terminate(State, Req, HandlerState, {error, closed});
 		{Error, Socket, Reason} ->
 		{Error, Socket, Reason} ->
 			handler_terminate(State, Req, HandlerState, {error, Reason});
 			handler_terminate(State, Req, HandlerState, {error, Reason});
+		{?MODULE, timeout, TRef} ->
+			websocket_close(State, Req, HandlerState, {normal, timeout});
+		{?MODULE, timeout, OlderTRef} when is_reference(OlderTRef) ->
+			handler_loop(State, Req, HandlerState, SoFar);
 		Message ->
 		Message ->
 			handler_call(State, Req, HandlerState,
 			handler_call(State, Req, HandlerState,
 				SoFar, websocket_info, Message, fun handler_before_loop/4)
 				SoFar, websocket_info, Message, fun handler_before_loop/4)
-	after Timeout ->
-		websocket_close(State, Req, HandlerState, {normal, timeout})
 	end.
 	end.
 
 
 -spec websocket_data(#state{}, #http_req{}, any(), binary()) -> ok.
 -spec websocket_data(#state{}, #http_req{}, any(), binary()) -> ok.

+ 31 - 2
test/http_SUITE.erl

@@ -19,7 +19,8 @@
 -export([all/0, groups/0, init_per_suite/1, end_per_suite/1,
 -export([all/0, groups/0, init_per_suite/1, end_per_suite/1,
 	init_per_group/2, end_per_group/2]). %% ct.
 	init_per_group/2, end_per_group/2]). %% ct.
 -export([chunked_response/1, headers_dupe/1, headers_huge/1,
 -export([chunked_response/1, headers_dupe/1, headers_huge/1,
-	keepalive_nl/1, nc_rand/1, pipeline/1, raw/1, ws0/1, ws8/1]). %% http.
+	keepalive_nl/1, nc_rand/1, pipeline/1, raw/1,
+	ws0/1, ws8/1, ws_timeout_hibernate/1]). %% http.
 -export([http_200/1, http_404/1]). %% http and https.
 -export([http_200/1, http_404/1]). %% http and https.
 -export([http_10_hostless/1]). %% misc.
 -export([http_10_hostless/1]). %% misc.
 
 
@@ -31,7 +32,8 @@ all() ->
 groups() ->
 groups() ->
 	BaseTests = [http_200, http_404],
 	BaseTests = [http_200, http_404],
 	[{http, [], [chunked_response, headers_dupe, headers_huge,
 	[{http, [], [chunked_response, headers_dupe, headers_huge,
-		keepalive_nl, nc_rand, pipeline, raw, ws0, ws8] ++ BaseTests},
+		keepalive_nl, nc_rand, pipeline, raw,
+		ws0, ws8, ws_timeout_hibernate] ++ BaseTests},
 	{https, [], BaseTests}, {misc, [], [http_10_hostless]}].
 	{https, [], BaseTests}, {misc, [], [http_10_hostless]}].
 
 
 init_per_suite(Config) ->
 init_per_suite(Config) ->
@@ -90,6 +92,7 @@ init_http_dispatch() ->
 		{[<<"localhost">>], [
 		{[<<"localhost">>], [
 			{[<<"chunked_response">>], chunked_handler, []},
 			{[<<"chunked_response">>], chunked_handler, []},
 			{[<<"websocket">>], websocket_handler, []},
 			{[<<"websocket">>], websocket_handler, []},
+			{[<<"ws_timeout_hibernate">>], ws_timeout_hibernate_handler, []},
 			{[<<"headers">>, <<"dupe">>], http_handler,
 			{[<<"headers">>, <<"dupe">>], http_handler,
 				[{headers, [{<<"Connection">>, <<"close">>}]}]},
 				[{headers, [{<<"Connection">>, <<"close">>}]}]},
 			{[], http_handler, []}
 			{[], http_handler, []}
@@ -297,6 +300,32 @@ ws8(Config) ->
 	{error, closed} = gen_tcp:recv(Socket, 0, 6000),
 	{error, closed} = gen_tcp:recv(Socket, 0, 6000),
 	ok.
 	ok.
 
 
+ws_timeout_hibernate(Config) ->
+	{port, Port} = lists:keyfind(port, 1, Config),
+	{ok, Socket} = gen_tcp:connect("localhost", Port,
+		[binary, {active, false}, {packet, raw}]),
+	ok = gen_tcp:send(Socket, [
+		"GET /ws_timeout_hibernate HTTP/1.1\r\n"
+		"Host: localhost\r\n"
+		"Connection: Upgrade\r\n"
+		"Upgrade: websocket\r\n"
+		"Sec-WebSocket-Origin: http://localhost\r\n"
+		"Sec-WebSocket-Version: 8\r\n"
+		"Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n"
+		"\r\n"]),
+	{ok, Handshake} = gen_tcp:recv(Socket, 0, 6000),
+	{ok, {http_response, {1, 1}, 101, "Switching Protocols"}, Rest}
+		= erlang:decode_packet(http, Handshake, []),
+	[Headers, <<>>] = websocket_headers(
+		erlang:decode_packet(httph, Rest, []), []),
+	{'Connection', "Upgrade"} = lists:keyfind('Connection', 1, Headers),
+	{'Upgrade', "websocket"} = lists:keyfind('Upgrade', 1, Headers),
+	{"sec-websocket-accept", "s3pPLMBiTxaQ9kYGzzhZRbK+xOo="}
+		= lists:keyfind("sec-websocket-accept", 1, Headers),
+	{ok, << 1:1, 0:3, 8:4, 0:8 >>} = gen_tcp:recv(Socket, 0, 6000),
+	{error, closed} = gen_tcp:recv(Socket, 0, 6000),
+	ok.
+
 websocket_headers({ok, http_eoh, Rest}, Acc) ->
 websocket_headers({ok, http_eoh, Rest}, Acc) ->
 	[Acc, Rest];
 	[Acc, Rest];
 websocket_headers({ok, {http_header, _I, Key, _R, Value}, Rest}, Acc) ->
 websocket_headers({ok, {http_header, _I, Key, _R, Value}, Rest}, Acc) ->

+ 29 - 0
test/ws_timeout_hibernate_handler.erl

@@ -0,0 +1,29 @@
+%% Feel free to use, reuse and abuse the code in this file.
+
+-module(ws_timeout_hibernate_handler).
+-behaviour(cowboy_http_handler).
+-behaviour(cowboy_http_websocket_handler).
+-export([init/3, handle/2, terminate/2]).
+-export([websocket_init/3, websocket_handle/3,
+	websocket_info/3, websocket_terminate/3]).
+
+init(_Any, _Req, _Opts) ->
+	{upgrade, protocol, cowboy_http_websocket}.
+
+handle(_Req, _State) ->
+	exit(badarg).
+
+terminate(_Req, _State) ->
+	exit(badarg).
+
+websocket_init(_TransportName, Req, _Opts) ->
+	{ok, Req, undefined, 1000, hibernate}.
+
+websocket_handle(_Frame, Req, State) ->
+	{ok, Req, State, hibernate}.
+
+websocket_info(_Info, Req, State) ->
+	{ok, Req, State, hibernate}.
+
+websocket_terminate(_Reason, _Req, _State) ->
+	ok.