Fix handler crashes handling

We try to send a 500 error only if we didn't send the response
headers yet. If they were, then we have no way to be sure the
response was fully sent, nor should we assume anything about
how this will be handled client-side, so we do nothing more
and in both cases close the connection.

3 files changed, 30 insertions, 11 deletions

diff --git a/src/cowboy_http_protocol.erl b/src/cowboy_http_protocol.erl
index 9b71e6c..ea59799 100644
--- a/src/cowboy_http_protocol.erl
+++ b/src/cowboy_http_protocol.erl
@@ -120,13 +120,13 @@ request({http_request, Method, {abs_path, AbsPath}, Version},
 	{Path, RawPath, Qs} = cowboy_dispatcher:split_path(AbsPath, URLDecode),
 	ConnAtom = version_to_connection(Version),
 	parse_header(#http_req{socket=Socket, transport=Transport,
-		connection=ConnAtom, method=Method, version=Version,
+		connection=ConnAtom, pid=self(), method=Method, version=Version,
 		path=Path, raw_path=RawPath, raw_qs=Qs, urldecode=URLDec}, State);
 request({http_request, Method, '*', Version},
 		State=#state{socket=Socket, transport=Transport, urldecode=URLDec}) ->
 	ConnAtom = version_to_connection(Version),
 	parse_header(#http_req{socket=Socket, transport=Transport,
-		connection=ConnAtom, method=Method, version=Version,
+		connection=ConnAtom, pid=self(), method=Method, version=Version,
 		path='*', raw_path= <<"*">>, raw_qs= <<>>, urldecode=URLDec}, State);
 request({http_request, _Method, _URI, _Version}, State) ->
 	error_terminate(501, State);
@@ -276,7 +276,7 @@ handler_handle(HandlerState, Req, State=#state{handler={Handler, Opts}}) ->
 			[Handler, Class, Reason, Opts,
 			 HandlerState, Req, erlang:get_stacktrace()]),
 		handler_terminate(HandlerState, Req, State),
-		terminate(State)
+		error_terminate(500, State)
 	end.
 
 %% We don't listen for Transport closes because that would force us
@@ -331,7 +331,9 @@ handler_call(HandlerState, Req, State=#state{handler={Handler, Opts}},
 			"** 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()]),
+		handler_terminate(HandlerState, Req, State),
+		error_terminate(500, State)
 	end.
 
 -spec handler_terminate(any(), #http_req{}, #state{}) -> ok.
@@ -359,6 +361,8 @@ next_request(Req=#http_req{connection=Conn, buffer=Buffer},
 		HandlerRes) ->
 	RespRes = ensure_response(Req),
 	BodyRes = ensure_body_processed(Req),
+	%% Flush the resp_sent message before moving on.
+	receive {cowboy_http_req, resp_sent} -> ok after 0 -> ok end,
 	case {HandlerRes, BodyRes, RespRes, Conn} of
 		{ok, ok, ok, keepalive} when Keepalive < MaxKeepalive ->
 			?MODULE:parse_request(State#state{
@@ -395,11 +399,17 @@ ensure_response(#http_req{socket=Socket, transport=Transport,
 	Transport:send(Socket, <<"0\r\n\r\n">>),
 	close.
 
+%% Only send an error reply if there is no resp_sent message.
 -spec error_terminate(http_status(), #state{}) -> ok.
 error_terminate(Code, State=#state{socket=Socket, transport=Transport}) ->
-	_ = cowboy_http_req:reply(Code, [], [], #http_req{
-		socket=Socket, transport=Transport,
-		connection=close, resp_state=waiting}),
+	receive
+		{cowboy_http_req, resp_sent} -> ok
+	after 0 ->
+		_ = cowboy_http_req:reply(Code, #http_req{
+			socket=Socket, transport=Transport,
+			connection=close, pid=self(), resp_state=waiting}),
+		ok
+	end,
 	terminate(State).
 
 -spec terminate(#state{}) -> ok.
diff --git a/src/cowboy_http_req.erl b/src/cowboy_http_req.erl
index 7a0e3a7..b0a0232 100644
--- a/src/cowboy_http_req.erl
+++ b/src/cowboy_http_req.erl
@@ -473,7 +473,7 @@ reply(Status, Headers, Req=#http_req{resp_body=Body}) ->
 -spec reply(http_status(), http_headers(), iodata(), #http_req{})
 	-> {ok, #http_req{}}.
 reply(Status, Headers, Body, Req=#http_req{socket=Socket,
-		transport=Transport, connection=Connection,
+		transport=Transport, connection=Connection, pid=ReqPid,
 		method=Method, resp_state=waiting, resp_headers=RespHeaders}) ->
 	RespConn = response_connection(Headers, Connection),
 	ContentLen = case Body of {CL, _} -> CL; _ -> iolist_size(Body) end,
@@ -488,6 +488,7 @@ reply(Status, Headers, Body, Req=#http_req{socket=Socket,
 		{_, {_, StreamFun}} -> Transport:send(Socket, Head), StreamFun();
 		{_, _} -> Transport:send(Socket, [Head, Body])
 	end,
+	ReqPid ! {?MODULE, resp_sent},
 	{ok, Req#http_req{connection=RespConn, resp_state=done,
 		resp_headers=[], resp_body= <<>>}}.
 
@@ -500,8 +501,9 @@ chunked_reply(Status, Req) ->
 %% @see cowboy_http_req:chunk/2
 -spec chunked_reply(http_status(), http_headers(), #http_req{})
 	-> {ok, #http_req{}}.
-chunked_reply(Status, Headers, Req=#http_req{socket=Socket, transport=Transport,
-		connection=Connection, resp_state=waiting, resp_headers=RespHeaders}) ->
+chunked_reply(Status, Headers, Req=#http_req{socket=Socket,
+		transport=Transport, connection=Connection, pid=ReqPid,
+		resp_state=waiting, resp_headers=RespHeaders}) ->
 	RespConn = response_connection(Headers, Connection),
 	Head = response_head(Status, Headers, RespHeaders, [
 		{<<"Connection">>, atom_to_connection(Connection)},
@@ -510,6 +512,7 @@ chunked_reply(Status, Headers, Req=#http_req{socket=Socket, transport=Transport,
 		{<<"Server">>, <<"Cowboy">>}
 	]),
 	Transport:send(Socket, Head),
+	ReqPid ! {?MODULE, resp_sent},
 	{ok, Req#http_req{connection=RespConn, resp_state=chunks,
 		resp_headers=[], resp_body= <<>>}}.
 
@@ -524,14 +527,16 @@ chunk(Data, #http_req{socket=Socket, transport=Transport, resp_state=chunks}) ->
 		<<"\r\n">>, Data, <<"\r\n">>]).
 
 %% @doc Send an upgrade reply.
+%% @private
 -spec upgrade_reply(http_status(), http_headers(), #http_req{})
 	-> {ok, #http_req{}}.
 upgrade_reply(Status, Headers, Req=#http_req{socket=Socket, transport=Transport,
-		resp_state=waiting, resp_headers=RespHeaders}) ->
+		pid=ReqPid, resp_state=waiting, resp_headers=RespHeaders}) ->
 	Head = response_head(Status, Headers, RespHeaders, [
 		{<<"Connection">>, <<"Upgrade">>}
 	]),
 	Transport:send(Socket, Head),
+	ReqPid ! {?MODULE, resp_sent},
 	{ok, Req#http_req{resp_state=done, resp_headers=[], resp_body= <<>>}}.
 
 %% Misc API.
diff --git a/src/cowboy_http_websocket.erl b/src/cowboy_http_websocket.erl
index 058c843..0f0204c 100644
--- a/src/cowboy_http_websocket.erl
+++ b/src/cowboy_http_websocket.erl
@@ -169,6 +169,8 @@ websocket_handshake(State=#state{version=0, origin=Origin,
 		 {<<"Sec-Websocket-Location">>, Location},
 		 {<<"Sec-Websocket-Origin">>, Origin}],
 		Req#http_req{resp_state=waiting}),
+	%% Flush the resp_sent message before moving on.
+	receive {cowboy_http_req, resp_sent} -> ok after 0 -> ok end,
 	%% We replied with a proper response. Proxies should be happy enough,
 	%% we can now read the 8 last bytes of the challenge keys and send
 	%% the challenge response directly to the socket.
@@ -188,6 +190,8 @@ websocket_handshake(State=#state{challenge=Challenge},
 		[{<<"Upgrade">>, <<"websocket">>},
 		 {<<"Sec-Websocket-Accept">>, Challenge}],
 		Req#http_req{resp_state=waiting}),
+	%% Flush the resp_sent message before moving on.
+	receive {cowboy_http_req, resp_sent} -> ok after 0 -> ok end,
 	handler_before_loop(State#state{messages=Transport:messages()},
 		Req2, HandlerState, <<>>).