Browse Source

Fix {cowboy_req, resp_sent} potentially leaking in loop handlers

Loïc Hoguin 12 years ago
parent
commit
638638a841
2 changed files with 18 additions and 4 deletions
  1. 17 4
      src/cowboy_handler.erl
  2. 1 0
      src/cowboy_websocket.erl

+ 17 - 4
src/cowboy_handler.erl

@@ -29,7 +29,8 @@
 	env :: cowboy_middleware:env(),
 	env :: cowboy_middleware:env(),
 	hibernate = false :: boolean(),
 	hibernate = false :: boolean(),
 	loop_timeout = infinity :: timeout(),
 	loop_timeout = infinity :: timeout(),
-	loop_timeout_ref :: undefined | reference()
+	loop_timeout_ref :: undefined | reference(),
+	resp_sent = false :: boolean()
 }).
 }).
 
 
 %% @private
 %% @private
@@ -80,7 +81,7 @@ handler_init(Req, State, Handler, HandlerOpts) ->
 			"** Stacktrace: ~p~n~n",
 			"** Stacktrace: ~p~n~n",
 			[Handler, init, 3, Class, Reason, HandlerOpts,
 			[Handler, init, 3, Class, Reason, HandlerOpts,
 				cowboy_req:to_list(Req), erlang:get_stacktrace()]),
 				cowboy_req:to_list(Req), erlang:get_stacktrace()]),
-		{error, 500, Req}
+		error_terminate(Req, State)
 	end.
 	end.
 
 
 -spec upgrade_protocol(Req, #state{}, module(), any(), module())
 -spec upgrade_protocol(Req, #state{}, module(), any(), module())
@@ -112,7 +113,7 @@ handler_handle(Req, State, Handler, HandlerState) ->
 			[Handler, handle, 2, Class, Reason, HandlerState,
 			[Handler, handle, 2, Class, Reason, HandlerState,
 				cowboy_req:to_list(Req), erlang:get_stacktrace()]),
 				cowboy_req:to_list(Req), erlang:get_stacktrace()]),
 		handler_terminate(Req, Handler, HandlerState, Reason),
 		handler_terminate(Req, Handler, HandlerState, Reason),
-		{error, 500, Req}
+		error_terminate(Req, State)
 	end.
 	end.
 
 
 %% We don't listen for Transport closes because that would force us
 %% We don't listen for Transport closes because that would force us
@@ -147,6 +148,9 @@ handler_loop_timeout(State=#state{loop_timeout=Timeout,
 	when Req::cowboy_req:req().
 	when Req::cowboy_req:req().
 handler_loop(Req, State=#state{loop_timeout_ref=TRef}, Handler, HandlerState) ->
 handler_loop(Req, State=#state{loop_timeout_ref=TRef}, Handler, HandlerState) ->
 	receive
 	receive
+		{cowboy_req, resp_sent} ->
+			handler_loop(Req, State#state{resp_sent=true},
+				Handler, HandlerState);
 		{timeout, TRef, ?MODULE} ->
 		{timeout, TRef, ?MODULE} ->
 			terminate_request(Req, State, Handler, HandlerState,
 			terminate_request(Req, State, Handler, HandlerState,
 				{normal, timeout});
 				{normal, timeout});
@@ -180,7 +184,7 @@ handler_call(Req, State, Handler, HandlerState, Message) ->
 			[Handler, info, 3, Class, Reason, HandlerState,
 			[Handler, info, 3, Class, Reason, HandlerState,
 				cowboy_req:to_list(Req), erlang:get_stacktrace()]),
 				cowboy_req:to_list(Req), erlang:get_stacktrace()]),
 		handler_terminate(Req, Handler, HandlerState, Reason),
 		handler_terminate(Req, Handler, HandlerState, Reason),
-		{error, 500, Req}
+		error_terminate(Req, State)
 	end.
 	end.
 
 
 -spec terminate_request(Req, #state{}, module(), any(),
 -spec terminate_request(Req, #state{}, module(), any(),
@@ -205,3 +209,12 @@ handler_terminate(Req, Handler, HandlerState, Reason) ->
 			[Handler, terminate, 3, Class, Reason2, HandlerState,
 			[Handler, terminate, 3, Class, Reason2, HandlerState,
 				cowboy_req:to_list(Req), erlang:get_stacktrace()])
 				cowboy_req:to_list(Req), erlang:get_stacktrace()])
 	end.
 	end.
+
+%% Only send an error reply if there is no resp_sent message.
+-spec error_terminate(Req, #state{})
+	-> {error, 500, Req} | {halt, Req} when Req::cowboy_req:req().
+error_terminate(Req, #state{resp_sent=true}) ->
+	%% Close the connection, but do not attempt sending a reply.
+	{halt, cowboy_req:set([{connection, close}, {resp_state, done}], Req)};
+error_terminate(Req, _) ->
+	{error, 500, Req}.

+ 1 - 0
src/cowboy_websocket.erl

@@ -121,6 +121,7 @@ handler_init(State=#state{env=Env, transport=Transport,
 		upgrade_error(Req, Env)
 		upgrade_error(Req, Env)
 	end.
 	end.
 
 
+%% Only send an error reply if there is no resp_sent message.
 -spec upgrade_error(Req, Env) -> {ok, Req, Env} | {error, 400, Req}
 -spec upgrade_error(Req, Env) -> {ok, Req, Env} | {error, 400, Req}
 	when Req::cowboy_req:req(), Env::cowboy_middleware:env().
 	when Req::cowboy_req:req(), Env::cowboy_middleware:env().
 upgrade_error(Req, Env) ->
 upgrade_error(Req, Env) ->