Process leaks when using async requests

[bblaszkow06]

Whenever a request with stream_to: pid is made using HTTPosion a transformer process is started (here). Unfortunately, it is possible that this process will stay alive forever.

First case: when making a request to a nonexistent domain:

iex(1)> Process.info(self(), :links)
{:links, []}
iex(2)> HTTPoison.get("nonexistantdomain.dasdsadaadasdasd.com", [], stream_to: self())
{:error, %HTTPoison.Error{id: nil, reason: :nxdomain}}
iex(3)> Process.info(self(), :links)                                                  
{:links, [#PID<0.223.0>]}

Second case: closing an async response (obtained using async: :once setting) manually:

iex(1)> Process.info(self(), :links)                                                  
{:links, []}
iex(2)> {:ok, resp} = HTTPoison.get("google.com", [], async: :once, stream_to: self())
{:ok, %HTTPoison.AsyncResponse{id: #Reference<0.3392924140.3015966722.68493>}}
iex(3)> flush
%HTTPoison.AsyncStatus{code: 301, id: #Reference<0.3392924140.3015966722.68493>}
:ok
iex(4)> :hackney.close(resp.id)                                                       
:ok
iex(5)> flush                  
:ok
iex(6)> Process.info(self(), :links)                                                  
{:links, [#PID<0.223.0>]}

The last one I've found: when :hackney async response dies due to this bug

iex(1)> Process.info(self(), :links)
{:links, []}
iex(2)> url = "https://download.fedoraproject.org/pub/fedora/linux/releases/29/Workstation/x86_64/iso/Fedora-Workstation-Live-x86_64-29-1.2.iso"
"https://download.fedoraproject.org/pub/fedora/linux/releases/29/Workstation/x86_64/iso/Fedora-Workstation-Live-x86_64-29-1.2.iso"
iex(3)> {:ok, resp} = HTTPoison.get(url, [], async: :once, stream_to: self())                                                                   
{:ok, %HTTPoison.AsyncResponse{id: #Reference<0.4284178466.2212757508.4220>}}                                                                                                                              
15:41:28.207 [error] Unexpected message: {:ssl_closed, {:sslsocket, {:gen_tcp, #Port<0.7>, :tls_connection, :undefined}, [#PID<0.230.0>, #PID<0.229.0>]}}

iex(4)> flush                       
%HTTPoison.AsyncStatus{code: 302, id: #Reference<0.4284178466.2212757508.4220>}
:ok
iex(5)> HTTPoison.stream_next(resp)                                                                                                             
{:error,
 %HTTPoison.Error{
   id: #Reference<0.4284178466.2212757508.4220>,
   reason: "stream_next/1 failed"
 }}
iex(6)> Process.info(self(), :links)                                                                                                            
{:links, [#PID<0.224.0>]}

[edgurgel]

Yes we need a better approach to the "transformer". Currently we spawn with no link, no monitor. I wonder if most users would prefer to crash if one of requests fail completely? 🤔

[edgurgel]

Thanks for creating the issue!

[bblaszkow06]

Actually, the transformer is spawned with a link to a process making the request. If I understand correctly, and you want to crash that process, then no I don't think users would prefer that 😉 I can imagine a scenario, where there is a GenServer responsible for polling some service(s) with a list of alternative URLs - if one fails, it tries another one. Crashing that process would be far from preferred.

Looking at the code, here's what I would suggest:

• The request function should be responsible for killing transformer in case of an error
• pid of transformer could be placed inside the AsyncResponse struct, so in case of an error in stream_next the transformer can be killed
• As there is no way to monitor :hackney request, the use of :hackney.close(response.id) should be prohibited and a way to close the response that will terminate the transformer as well should be added to HTTPoison (closing Close an async request manually #103 BTW)
• To make sure that request-making process won't be crashed by a bug in a transformer, spawn_link
  could be replaced with spawn + monitor

I may be able to make a PR with the changes, but no ETA on that

[chazsconi]

I have a PR #395 which would partially solve a related issue when the target process dies or is killed. For my use case this solves the problem of zombie processes.