From 4cbdfdd293c687212f9ce64e608de838ec74a592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Sun, 5 Feb 2017 13:45:35 +0100 Subject: Fix sending of large files with HTTP/2 Also finish implementing the relevant test, getting rid of todos. --- src/cowboy_http2.erl | 22 ++++++++++++++++++++-- test/static_handler_SUITE.erl | 31 ++++++++++++++++++++++++------- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/cowboy_http2.erl b/src/cowboy_http2.erl index ead3ff5..e816c93 100644 --- a/src/cowboy_http2.erl +++ b/src/cowboy_http2.erl @@ -441,8 +441,13 @@ commands(State=#state{socket=Socket, transport=Transport}, Stream=#stream{id=Str %% implementation necessarily varies between HTTP/1.1 and HTTP/2. commands(State=#state{socket=Socket, transport=Transport}, Stream=#stream{id=StreamID, local=nofin}, [{sendfile, IsFin, Offset, Bytes, Path}|Tail]) -> - Transport:send(Socket, cow_http2:data_header(StreamID, IsFin, Bytes)), - Transport:sendfile(Socket, Path, Offset, Bytes), + %% @todo We currently have a naive implementation without a + %% scheduler to prioritize frames that need to be sent. + %% A future update will need to queue such data frames + %% and only send them when there is nothing currently + %% being sent. We would probably also benefit from doing + %% asynchronous sends. + sendfile(Socket, Transport, StreamID, IsFin, Offset, Bytes, Path, 16384), commands(State, Stream#stream{local=IsFin}, Tail); %% @todo sendfile when local!=nofin %% Send a push promise. @@ -513,6 +518,19 @@ send_data(Socket, Transport, StreamID, IsFin, Data, Length) -> Transport:send(Socket, cow_http2:data(StreamID, IsFin, Data)) end. +%% @todo This is currently awfully slow. But at least it's correct. +sendfile(Socket, Transport, StreamID, IsFin, Offset, Bytes, Path, Length) -> + if + Length < Bytes -> + Transport:send(Socket, cow_http2:data_header(StreamID, nofin, Length)), + Transport:sendfile(Socket, Path, Offset, Length), + sendfile(Socket, Transport, StreamID, IsFin, + Offset + Length, Bytes - Length, Path, Length); + true -> + Transport:send(Socket, cow_http2:data_header(StreamID, IsFin, Bytes)), + Transport:sendfile(Socket, Path, Offset, Bytes) + end. + -spec terminate(#state{}, _) -> no_return(). terminate(#state{socket=Socket, transport=Transport, streams=Streams, children=Children}, Reason) -> diff --git a/test/static_handler_SUITE.erl b/test/static_handler_SUITE.erl index 7a6517f..95d6d75 100644 --- a/test/static_handler_SUITE.erl +++ b/test/static_handler_SUITE.erl @@ -391,13 +391,30 @@ dir_html(Config) -> {_, <<"text/html">>} = lists:keyfind(<<"content-type">>, 1, Headers), ok. -%% @todo This test results in a crash dump. -%dir_large_file(Config) -> -% doc("Get a large file."), -% {200, Headers, _} = do_get(config(prefix, Config) ++ "/large.bin", Config), -% {_, <<"text/html">>} = lists:keyfind(<<"content-type">>, 1, Headers), -%% @todo Receive body. -% ok. +dir_large_file(Config) -> + doc("Get a large file."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, config(prefix, Config) ++ "/large.bin", + [{<<"accept-encoding">>, <<"gzip">>}]), + {response, nofin, 200, RespHeaders} = gun:await(ConnPid, Ref), + {_, <<"application/octet-stream">>} = lists:keyfind(<<"content-type">>, 1, RespHeaders), + Size = 512*1024*1024, + {ok, Size} = do_dir_large_file(ConnPid, Ref, 0), + ok. + +do_dir_large_file(ConnPid, Ref, N) -> + receive + {gun_data, ConnPid, Ref, nofin, Data} -> + do_dir_large_file(ConnPid, Ref, N + byte_size(Data)); + {gun_data, ConnPid, Ref, fin, Data} -> + {ok, N + byte_size(Data)}; + {gun_error, ConnPid, Ref, Reason} -> + {error, Reason}; + {gun_error, ConnPid, Reason} -> + {error, Reason} + after 5000 -> + {error, timeout} + end. dir_text(Config) -> doc("Get a .txt file. The extension is unknown by default."), -- cgit v1.2.3