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 https://github.com/benoitc/hackney/compare/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 notifications@github.com 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 https://github.com/benoitc/hackney/compare/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 https://github.com/benoitc/hackney/pull/640#issuecomment-751761715, 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