Fix ssl_closed message leak when server hangs up ("Connection: close" header)

[rozap]

This should fix #464

• mutually recursive maybe_continue and async_recv
  functions are really tricky to reason about, but
  it doesn't seem like it makes sense for maybe_continue
  to do error handling at all, since async_recv was already
  doing it

  • maybe_continue and stream_loop shouldn't select
    socket messages
  • we're getting into a state where the socket sends
    us a message while we're doing other things and
    we do that receive and then crash

• this change makes async_recv the only one to do
  a select for transport messages

• When the server closes the connection
  and :ssl sends us :ssl_closed, we weren't
  handling the message, which caused it to leak
  into the mailbox even after the request was ended

  • need to flush to make sure it's gone in this case
    where the server hangs up
  • caused when Connection: close header is sent

fix debugging statement

[erik-mocny]

@benoitc Hi, please why is it still not merged as a whole? I can see only second commit changes in 1.16.0...v1.17.0 probably cherry-picked? Thanks

[benoitc]

i didn't cherry-picked :) I merged your other PR #641 . I didn't merge this one yet as i wanted to test it first. There will be a new release next week that should fix the issue.

[erik-mocny]

Thanks for the reply. It wasn't my PR I just asked about it because we were facing same issue in our projects.

[Dvogiatz]

Hello @benoitc, what is the current status on this PR?

There will be a new release next week that should fix the issue.

Also is this referring to a 1.17.1 version coming soon? Because with 1.17.0 which mentions this PR, the issue still exists. Thank
you

edit* I cloned the 1.17.0 master branch and cherry picked this PR and I can confirm that it solved the issue.

[Dvogiatz]

Update: after 2 days of working properly without any issues, it started to throw the same error again.

[benoitc]

Sorry to have been silent. There is a new release coming next week that will fix this issue. I will create a ticket summarizing it.

…

On Mon 28 Dec 2020 at 16:56 Erik Mocny ***@***.***> wrote: @benoitc <https://github.com/benoitc> Hi, please why is it still not merged as a whole? I can see only second commit changes in 1.16.0...v1.17.0 <1.16.0...v1.17.0> probably cherry-picked? Thanks — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#640 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAADRIQMU6533TEVVYHWHXTSXCTCLANCNFSM4N4QYHCA> .

[luizmiranda7]

@benoitc I'm trying the new version of hackney: 1.17.4 but it keeps the same behavior:

{:ssl_closed,
 {:sslsocket, {:gen_tcp, #Port<0.330>, :tls_connection, :undefined}

• Erlang/OTP 22 [erts-10.7.2.7]
• Hackney 1.17.4

[ringofhealth]

 [error] Unexpected message: {:ssl_closed, {:sslsocket, {:gen_tcp, #Port<0.213>, :tls_connection, :undefined}, [#PID<0.2999.0>, #PID<0.2998.0>]}}

same issue here as well

Erlang/OTP 22 [erts-10.7.2.7]
Hackney 1.17.4

here is the function implementation

  def get(url) do
    Stream.resource(
      # start_fun
      fn ->
        HTTPoison.get!(
          url,
          %{},
          stream_to: self(),
          async: :once,
          hackney: [
            ssl_options: [
              versions: [:"tlsv1.2"]
            ]
          ]
        )
      end,
      fn %HTTPoison.AsyncResponse{id: id} = resp ->
        receive do
          %HTTPoison.AsyncStatus{id: ^id, code: _code} ->
            HTTPoison.stream_next(resp)
            {[], resp}

          %HTTPoison.AsyncHeaders{id: ^id, headers: _headers} ->
            HTTPoison.stream_next(resp)
            {[], resp}

          %HTTPoison.AsyncChunk{id: ^id, chunk: chunk} ->
            HTTPoison.stream_next(resp)
            {[chunk], resp}

          %HTTPoison.AsyncEnd{id: ^id} ->
            {:halt, resp}
        end
      end,
      fn _end_func ->
        nil
      end
    )
  end