Browse Source

Move recursion out of a try .. catch block.

Fixes issue #31.

Recursion shouldn't happen in a single catch statement or inside
a try .. catch statement. The only safe construct for catching
exceptions and perform recursion when everything goes well is
to put the recursive call inside a try .. of .. catch construct
in the of .. catch block.

Otherwise the stack gets filled with exception-related information
since they can still be caught if we were to send them and unfold
the whole thing.

Thanks go to lpgauth for reporting the issue and people on IRC
for explaining the hows and whys.
Loïc Hoguin 13 years ago
parent
commit
aa0a66749e
1 changed files with 12 additions and 13 deletions
  1. 12 13
      src/cowboy_http_protocol.erl

+ 12 - 13
src/cowboy_http_protocol.erl

@@ -223,25 +223,24 @@ handler_loop(HandlerState, Req, State=#state{handler={Handler, Opts}}) ->
 -spec handler_terminate(any(), #http_req{}, #state{}) -> ok.
 handler_terminate(HandlerState, Req=#http_req{buffer=Buffer},
 		State=#state{handler={Handler, Opts}}) ->
-	try
-		HandlerRes = Handler:terminate(Req#http_req{resp_state=locked},
-			HandlerState),
-		BodyRes = ensure_body_processed(Req),
-		RespRes = ensure_response(Req, State),
-		case {HandlerRes, BodyRes, RespRes, State#state.connection} of
-			{ok, ok, ok, keepalive} ->
-				?MODULE:parse_request(State#state{buffer=Buffer});
-			_Closed ->
-				terminate(State)
-		end
+	HandlerRes = try
+		Handler:terminate(Req#http_req{resp_state=locked}, HandlerState)
 	catch Class:Reason ->
-		terminate(State),
 		error_logger:error_msg(
 			"** Handler ~p terminating in terminate/2 for the reason ~p:~p~n"
 			"** Options were ~p~n** Handler state was ~p~n"
 			"** Request was ~p~n** Stacktrace: ~p~n~n",
 			[Handler, Class, Reason, Opts,
-			 HandlerState, Req, erlang:get_stacktrace()])
+			 HandlerState, Req, erlang:get_stacktrace()]),
+		error
+	end,
+	BodyRes = ensure_body_processed(Req),
+	RespRes = ensure_response(Req, State),
+	case {HandlerRes, BodyRes, RespRes, State#state.connection} of
+		{ok, ok, ok, keepalive} ->
+			?MODULE:parse_request(State#state{buffer=Buffer});
+		_Closed ->
+			terminate(State)
 	end.
 
 -spec ensure_body_processed(#http_req{}) -> ok | close.