Skip to content

Commit

Permalink
Ensure HTTP/1.1 Websocket resets the trap_exit flag
Browse files Browse the repository at this point in the history
While we are identified as a supervisor in the tree,
we no longer manage children processes at that point,
so do not need to trap exit signals. Users can still
enable trap_exit if they prefer to.
  • Loading branch information
essen committed Jan 8, 2024
1 parent 9784179 commit c1490d7
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 2 deletions.
1 change: 1 addition & 0 deletions doc/src/manual/cowboy_websocket.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ normal circumstances if necessary.

== Changelog

* *2.11*: HTTP/1.1 Websocket no longer traps exits by default.
* *2.8*: The `active_n` option was added.
* *2.7*: The commands based interface has been documented.
The old interface is now deprecated.
Expand Down
4 changes: 3 additions & 1 deletion src/cowboy_http.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1212,11 +1212,13 @@ commands(State0=#state{ref=Ref, parent=Parent, socket=Socket, transport=Transpor
_ -> State
end,
#stream{state=StreamState} = lists:keyfind(StreamID, #stream.id, Streams),
%% @todo We need to shutdown processes here first.
stream_call_terminate(StreamID, switch_protocol, StreamState, State),
%% Terminate children processes and flush any remaining messages from the mailbox.
cowboy_children:terminate(Children),
flush(Parent),
%% Turn off the trap_exit process flag
%% since this process will no longer be a supervisor.
process_flag(trap_exit, false),
Protocol:takeover(Parent, Ref, Socket, Transport, Opts, Buffer, InitialState);
%% Set options dynamically.
commands(State0=#state{overriden_opts=Opts},
Expand Down
5 changes: 4 additions & 1 deletion test/handlers/ws_init_h.erl
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ do_websocket_init(State=reply_many_hibernate) ->
do_websocket_init(State=reply_many_close) ->
{[{text, "Hello"}, close], State};
do_websocket_init(State=reply_many_close_hibernate) ->
{[{text, "Hello"}, close], State, hibernate}.
{[{text, "Hello"}, close], State, hibernate};
do_websocket_init(State=reply_trap_exit) ->
Text = "trap_exit: " ++ atom_to_list(element(2, process_info(self(), trap_exit))),
{[{text, Text}, close], State, hibernate}.

websocket_handle(_, State) ->
{[], State}.
Expand Down
1 change: 1 addition & 0 deletions test/ws_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ ws_first_frame_with_handshake(Config) ->
{ok, <<1:1, 0:3, 1:4, 0:1, 5:7, "Hello">>} = gen_tcp:recv(Socket, 0, 6000),
ok.

%% @todo Move these tests to ws_handler_SUITE.
ws_init_return_ok(Config) ->
doc("Handler does nothing."),
{ok, Socket, _} = do_handshake("/ws_init?ok", Config),
Expand Down
8 changes: 8 additions & 0 deletions test/ws_handler_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ init_dispatch(Name) ->
{"/init", ws_init_commands_h, RunOrHibernate},
{"/handle", ws_handle_commands_h, RunOrHibernate},
{"/info", ws_info_commands_h, RunOrHibernate},
{"/trap_exit", ws_init_h, RunOrHibernate},
{"/active", ws_active_commands_h, RunOrHibernate},
{"/deflate", ws_deflate_commands_h, RunOrHibernate},
{"/set_options", ws_set_options_commands_h, RunOrHibernate},
Expand Down Expand Up @@ -211,6 +212,13 @@ do_many_frames_then_close_frame(Config, Path) ->
{ok, close} = receive_ws(ConnPid, StreamRef),
gun_down(ConnPid).

websocket_init_trap_exit_false(Config) ->
doc("The trap_exit process flag must be set back to false before "
"the connection is taken over by Websocket."),
{ok, ConnPid, StreamRef} = gun_open_ws(Config, "/trap_exit?reply_trap_exit", []),
{ok, {text, <<"trap_exit: false">>}} = receive_ws(ConnPid, StreamRef),
ok.

websocket_active_false(Config) ->
doc("The {active, false} command stops receiving data from the socket. "
"The {active, true} command reenables it."),
Expand Down

0 comments on commit c1490d7

Please sign in to comment.