Browse Source

Exit gracefully on parent exit/sys:terminate/2,3

Loïc Hoguin 6 years ago
parent
commit
d4129e6305
5 changed files with 11 additions and 17 deletions
  1. 2 1
      doc/src/manual/cowboy_stream.asciidoc
  2. 2 4
      src/cowboy_http.erl
  3. 2 7
      src/cowboy_http2.erl
  4. 1 1
      src/cowboy_stream.erl
  5. 4 4
      test/sys_SUITE.erl

+ 2 - 1
doc/src/manual/cowboy_stream.asciidoc

@@ -386,7 +386,7 @@ reason() :: normal | switch_protocol
     | {socket_error, closed | atom(), HumanReadable}
     | {stream_error, Error, HumanReadable}
     | {connection_error, Error, HumanReadable}
-    | {stop, cow_http2:frame(), HumanReadable}
+    | {stop, cow_http2:frame() | {exit, any()}, HumanReadable}
 
 Error         = atom()
 HumanReadable = atom()
@@ -420,6 +420,7 @@ tuple.
 
 == Changelog
 
+* *2.6*: The `{stop, {exit, any()}, HumanReadable}` terminate reason was added.
 * *2.2*: The trailers command was introduced.
 * *2.0*: Module introduced.
 

+ 2 - 4
src/cowboy_http.erl

@@ -209,8 +209,7 @@ loop(State=#state{parent=Parent, socket=Socket, transport=Transport, opts=Opts,
 			loop(State, Buffer);
 		%% System messages.
 		{'EXIT', Parent, Reason} ->
-			%% @todo We should exit gracefully, if possible.
-			exit(Reason);
+			terminate(State, {stop, {exit, Reason}, 'Parent process terminated.'});
 		{system, From, Request} ->
 			sys:handle_system_msg(Request, From, Parent, ?MODULE, [], {State, Buffer});
 		%% Messages pertaining to a stream.
@@ -1379,8 +1378,7 @@ system_continue(_, _, {State, Buffer}) ->
 
 -spec system_terminate(any(), _, _, {#state{}, binary()}) -> no_return().
 system_terminate(Reason, _, _, {State, _}) ->
-	%% @todo We should exit gracefully, if possible.
-	terminate(State, Reason).
+	terminate(State, {stop, {exit, Reason}, 'sys:terminate/2,3 was called.'}).
 
 -spec system_code_change(Misc, _, _, _) -> {ok, Misc} when Misc::{#state{}, binary()}.
 system_code_change(Misc, _, _, _) ->

+ 2 - 7
src/cowboy_http2.erl

@@ -165,8 +165,7 @@ loop(State=#state{parent=Parent, socket=Socket, transport=Transport,
 			terminate(State, {socket_error, Reason, 'An error has occurred on the socket.'});
 		%% System messages.
 		{'EXIT', Parent, Reason} ->
-			%% @todo We should exit gracefully.
-			exit(Reason);
+			terminate(State, {stop, {exit, Reason}, 'Parent process terminated.'});
 		{system, From, Request} ->
 			sys:handle_system_msg(Request, From, Parent, ?MODULE, [], {State, Buffer});
 		%% Timeouts.
@@ -239,7 +238,6 @@ frame(State=#state{http2_machine=HTTP2Machine0}, Frame) ->
 				{stop, Frame, 'Client is going away.'});
 		{send, SendData, HTTP2Machine} ->
 			send_data(maybe_ack(State#state{http2_machine=HTTP2Machine}, Frame), SendData);
-		%% @todo Yeah StreamID would be better out of this, {error, StreamID, Tuple, State}
 		{error, {stream_error, StreamID, Reason, Human}, HTTP2Machine} ->
 			stream_reset(State#state{http2_machine=HTTP2Machine},
 				StreamID, {stream_error, Reason, Human});
@@ -591,8 +589,6 @@ send_response(State0, StreamID, StatusCode, Headers, Body) ->
 	end,
 	case Size of
 		0 ->
-			%% @todo If IsFin=fin, we may want to skip the body and terminate the stream,
-			%% if we are no longer tracking the stream.
 			State = send_headers(State0, StreamID, fin, StatusCode, Headers),
 			maybe_terminate_stream(State, StreamID, fin);
 		_ ->
@@ -783,8 +779,7 @@ system_continue(_, _, {State, Buffer}) ->
 
 -spec system_terminate(any(), _, _, {#state{}, binary()}) -> no_return().
 system_terminate(Reason, _, _, {State, _}) ->
-	%% @todo We should exit gracefully, if possible.
-	terminate(State, Reason).
+	terminate(State, {stop, {exit, Reason}, 'sys:terminate/2,3 was called.'}).
 
 -spec system_code_change(Misc, _, _, _) -> {ok, Misc} when Misc::{#state{}, binary()}.
 system_code_change(Misc, _, _, _) ->

+ 1 - 1
src/cowboy_stream.erl

@@ -50,7 +50,7 @@
 	| {socket_error, closed | atom(), human_reason()}
 	| {stream_error, cow_http2:error(), human_reason()}
 	| {connection_error, cow_http2:error(), human_reason()}
-	| {stop, cow_http2:frame(), human_reason()}.
+	| {stop, cow_http2:frame() | {exit, any()}, human_reason()}.
 -export_type([reason/0]).
 
 -type partial_req() :: map(). %% @todo Take what's in cowboy_req with everything? optional.

+ 4 - 4
test/sys_SUITE.erl

@@ -884,7 +884,7 @@ sys_suspend_and_resume_loop(Config) ->
 %% The callback Module:system_terminate/4 is used in all cases.
 
 sys_terminate_h1(Config) ->
-	doc("h1: The sys:terminate/1 function works as expected."),
+	doc("h1: The sys:terminate/2,3 function works as expected."),
 	{ok, Socket} = gen_tcp:connect("localhost", config(clear_port, Config), [{active, false}]),
 	timer:sleep(100),
 	Pid = get_remote_pid_tcp(Socket),
@@ -893,7 +893,7 @@ sys_terminate_h1(Config) ->
 	ok.
 
 sys_terminate_h2(Config) ->
-	doc("h2: The sys:terminate/1 function works as expected."),
+	doc("h2: The sys:terminate/2,3 function works as expected."),
 	{ok, Socket} = ssl:connect("localhost", config(tls_port, Config),
 		[{active, false}, binary, {alpn_advertised_protocols, [<<"h2">>]}]),
 	%% Skip the SETTINGS frame.
@@ -905,7 +905,7 @@ sys_terminate_h2(Config) ->
 	ok.
 
 sys_terminate_ws(Config) ->
-	doc("ws: The sys:terminate/1 function works as expected."),
+	doc("ws: The sys:terminate/2,3 function works as expected."),
 	{ok, Socket} = gen_tcp:connect("localhost", config(clear_port, Config),
 		[binary, {active, false}]),
 	ok = gen_tcp:send(Socket,
@@ -926,7 +926,7 @@ sys_terminate_ws(Config) ->
 	ok.
 
 sys_terminate_loop(Config) ->
-	doc("loop: The sys:terminate/1 function works as expected."),
+	doc("loop: The sys:terminate/2,3 function works as expected."),
 	{ok, Socket} = gen_tcp:connect("localhost", config(clear_port, Config), [{active, false}]),
 	ok = gen_tcp:send(Socket,
 		"GET /loop HTTP/1.1\r\n"