From 54e8284d883b11a83ec48edb41c9a15d657bae8e Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Tue, 2 Jul 2019 14:12:18 +0200 Subject: Fix extracting 0 bytes from queue --- lib/ssl/src/ssl_connection.erl | 2 ++ lib/ssl/src/tls_record.erl | 11 +++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index fbbe0a49c8..af9f0cbf3a 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -670,6 +670,8 @@ read_application_dist_data(DHandle, Front0, BufferSize, Rear0, Bin0) -> end end. +iovec_from_front(0, Front, Rear, Acc) -> + {lists:reverse(Acc),Front,Rear}; iovec_from_front(Size, [], Rear, Acc) -> iovec_from_front(Size, lists:reverse(Rear), [], Acc); iovec_from_front(Size, [Bin|Front], Rear, Acc) -> diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl index 38022030ee..afbf95948d 100644 --- a/lib/ssl/src/tls_record.erl +++ b/lib/ssl/src/tls_record.erl @@ -489,16 +489,19 @@ validate_tls_record_length(Versions, {_,Size0,_} = Q0, Acc, Type, Version, Lengt end. -binary_from_front(SplitSize, {Front,Size,Rear}) -> +binary_from_front(0, Q) -> + {<<>>, Q}; +binary_from_front(SplitSize, {Front,Size,Rear}) when SplitSize =< Size -> binary_from_front(SplitSize, Front, Size, Rear, []). %% +%% SplitSize > 0 and there is at least SplitSize bytes buffered in Front and Rear binary_from_front(SplitSize, [], Size, [_] = Rear, Acc) -> - %% Optimize a simple case + %% Optimize a simple case - avoid lists:reverse/1 binary_from_front(SplitSize, Rear, Size, [], Acc); -binary_from_front(SplitSize, [], Size, Rear, Acc) -> +binary_from_front(SplitSize, [], Size, [_|_] = Rear, Acc) -> binary_from_front(SplitSize, lists:reverse(Rear), Size, [], Acc); binary_from_front(SplitSize, [Bin|Front], Size, Rear, []) -> - %% Optimize a frequent case + %% Optimize the frequent case when the accumulator is empty BinSize = byte_size(Bin), if SplitSize < BinSize -> -- cgit v1.2.3 From bf0f197f566b688df0e0c6a5a343019643732d86 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Wed, 3 Jul 2019 11:47:57 +0200 Subject: Refine the queue code --- lib/ssl/src/ssl_connection.erl | 13 ++++++++++++- lib/ssl/src/tls_record.erl | 18 +++++++++++++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index af9f0cbf3a..b3b1dc18dc 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -673,7 +673,18 @@ read_application_dist_data(DHandle, Front0, BufferSize, Rear0, Bin0) -> iovec_from_front(0, Front, Rear, Acc) -> {lists:reverse(Acc),Front,Rear}; iovec_from_front(Size, [], Rear, Acc) -> - iovec_from_front(Size, lists:reverse(Rear), [], Acc); + case Rear of + %% Avoid lists:reverse/1 for simple cases. + %% Case clause for [] to avoid infinite loop. + [_] -> + iovec_from_front(Size, Rear, [], Acc); + [Bin2,Bin1] -> + iovec_from_front(Size, [Bin1,Bin2], [], Acc); + [Bin3,Bin2,Bin1] -> + iovec_from_front(Size, [Bin1,Bin2,Bin3], [], Acc); + [_,_,_|_] = Rear -> + iovec_from_front(Size, lists:reverse(Rear), [], Acc) + end; iovec_from_front(Size, [Bin|Front], Rear, Acc) -> case Bin of <> -> % Just enough diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl index afbf95948d..20598ea702 100644 --- a/lib/ssl/src/tls_record.erl +++ b/lib/ssl/src/tls_record.erl @@ -495,11 +495,19 @@ binary_from_front(SplitSize, {Front,Size,Rear}) when SplitSize =< Size -> binary_from_front(SplitSize, Front, Size, Rear, []). %% %% SplitSize > 0 and there is at least SplitSize bytes buffered in Front and Rear -binary_from_front(SplitSize, [], Size, [_] = Rear, Acc) -> - %% Optimize a simple case - avoid lists:reverse/1 - binary_from_front(SplitSize, Rear, Size, [], Acc); -binary_from_front(SplitSize, [], Size, [_|_] = Rear, Acc) -> - binary_from_front(SplitSize, lists:reverse(Rear), Size, [], Acc); +binary_from_front(SplitSize, [], Size, Rear, Acc) -> + case Rear of + %% Avoid lists:reverse/1 for simple cases. + %% Case clause for [] to avoid infinite loop. + [_] -> + binary_from_front(SplitSize, Rear, Size, [], Acc); + [Bin2,Bin1] -> + binary_from_front(SplitSize, [Bin1,Bin2], Size, [], Acc); + [Bin3,Bin2,Bin1] -> + binary_from_front(SplitSize, [Bin1,Bin2,Bin3], Size, [], Acc); + [_,_,_|_] -> + binary_from_front(SplitSize, lists:reverse(Rear), Size, [], Acc) + end; binary_from_front(SplitSize, [Bin|Front], Size, Rear, []) -> %% Optimize the frequent case when the accumulator is empty BinSize = byte_size(Bin), -- cgit v1.2.3 From 7db6faacd845ced366bbd98aef1ce85856781e53 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Fri, 5 Jul 2019 11:44:37 +0200 Subject: Do not call dist_ctrl_put_data with empty binaries --- lib/ssl/src/ssl_connection.erl | 42 +++++++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index b3b1dc18dc..8b3688f36d 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -610,7 +610,8 @@ read_application_dist_data(DHandle, Front0, BufferSize, Rear0, Bin0) -> <> -> + SizeD:32, DataD:SizeD/binary, Rest/binary>> + when 0 < SizeA, 0 < SizeB, 0 < SizeC, 0 < SizeD -> %% We have 4 complete packets in the first binary erlang:dist_ctrl_put_data(DHandle, DataA), erlang:dist_ctrl_put_data(DHandle, DataB), @@ -620,7 +621,8 @@ read_application_dist_data(DHandle, Front0, BufferSize, Rear0, Bin0) -> DHandle, Front0, BufferSize - (4*4+SizeA+SizeB+SizeC+SizeD), Rear0, Rest); <> -> + SizeC:32, DataC:SizeC/binary, Rest/binary>> + when 0 < SizeA, 0 < SizeB, 0 < SizeC -> %% We have 3 complete packets in the first binary erlang:dist_ctrl_put_data(DHandle, DataA), erlang:dist_ctrl_put_data(DHandle, DataB), @@ -628,7 +630,8 @@ read_application_dist_data(DHandle, Front0, BufferSize, Rear0, Bin0) -> read_application_dist_data( DHandle, Front0, BufferSize - (3*4+SizeA+SizeB+SizeC), Rear0, Rest); <> -> + SizeB:32, DataB:SizeB/binary, Rest/binary>> + when 0 < SizeA, 0 < SizeB -> %% We have 2 complete packets in the first binary erlang:dist_ctrl_put_data(DHandle, DataA), erlang:dist_ctrl_put_data(DHandle, DataB), @@ -639,13 +642,13 @@ read_application_dist_data(DHandle, Front0, BufferSize, Rear0, Bin0) -> %% Basic one packet code path <> -> %% We have a complete packet in the first binary - erlang:dist_ctrl_put_data(DHandle, Data), + 0 < Size andalso erlang:dist_ctrl_put_data(DHandle, Data), read_application_dist_data(DHandle, Front0, BufferSize - (4+Size), Rear0, Rest); <> when 4+Size =< BufferSize -> %% We have a complete packet in the buffer %% - fetch the missing content from the buffer front {Data,Front,Rear} = iovec_from_front(Size - byte_size(FirstData), Front0, Rear0, [FirstData]), - erlang:dist_ctrl_put_data(DHandle, Data), + 0 < Size andalso erlang:dist_ctrl_put_data(DHandle, Data), read_application_dist_data(DHandle, Front, BufferSize - (4+Size), Rear); <> -> %% In OTP-21 the match context reuse optimization fails if we use Bin0 in recursion, so here we @@ -661,12 +664,23 @@ read_application_dist_data(DHandle, Front0, BufferSize, Rear0, Bin0) -> %% contains enough data to maybe form a packet %% - fetch a tiny binary from the buffer front to complete the length field {LengthField,Front,Rear} = - iovec_from_front(4 - byte_size(IncompleteLengthField), Front0, Rear0, [IncompleteLengthField]), + case IncompleteLengthField of + <<>> -> + iovec_from_front(4, Front0, Rear0, []); + _ -> + iovec_from_front( + 4 - byte_size(IncompleteLengthField), Front0, Rear0, [IncompleteLengthField]) + end, LengthBin = iolist_to_binary(LengthField), read_application_dist_data(DHandle, Front, BufferSize, Rear, LengthBin); <> -> %% We do not have enough data in the buffer to even form a length field - await more data - {[IncompleteLengthField|Front0],BufferSize,Rear0} + case IncompleteLengthField of + <<>> -> + {Front0,BufferSize,Rear0}; + _ -> + {[IncompleteLengthField|Front0],BufferSize,Rear0} + end end end. @@ -685,12 +699,26 @@ iovec_from_front(Size, [], Rear, Acc) -> [_,_,_|_] = Rear -> iovec_from_front(Size, lists:reverse(Rear), [], Acc) end; +iovec_from_front(Size, [Bin|Front], Rear, []) -> + case Bin of + <> -> % Just enough + {[Last],Front,Rear}; + <> -> % More than enough, split here + {[Last],[Rest|Front],Rear}; + <<>> -> % Not enough, skip empty binaries + iovec_from_front(Size, Front, Rear, []); + <<_/binary>> -> % Not enough + BinSize = byte_size(Bin), + iovec_from_front(Size - BinSize, Front, Rear, [Bin]) + end; iovec_from_front(Size, [Bin|Front], Rear, Acc) -> case Bin of <> -> % Just enough {lists:reverse(Acc, [Last]),Front,Rear}; <> -> % More than enough, split here {lists:reverse(Acc, [Last]),[Rest|Front],Rear}; + <<>> -> % Not enough, skip empty binaries + iovec_from_front(Size, Front, Rear, Acc); <<_/binary>> -> % Not enough BinSize = byte_size(Bin), iovec_from_front(Size - BinSize, Front, Rear, [Bin|Acc]) -- cgit v1.2.3