JuliaWeb / HTTP.jl

HTTP for Julia
https://juliaweb.github.io/HTTP.jl/stable/
Other
626 stars 177 forks source link

How to close all cached connections #1137

Open DrChainsaw opened 6 months ago

DrChainsaw commented 6 months ago

Please specify the following versions when submitting a bug report:

Is there a way to explicitly close all connections, including the ones which are globally cached in Connections.jl?

There is Connections.closeall() but it only seems to empty the pools with the references but does not actually close the connections before doing. Could this be a bug or is it the intended behaviour for some reason? It seems like it would cause the connections to leak.

Background is that I have a package which does need to query a webpage as part of the computations it does (for reasons beyond my control) and when I use PrecompileTools I get warnings like this:

[pid 26608] waiting for IO to finish:
│   Handle type        uv_handle_t->data
│   tcp[864]           000002281cc4f140->0000022821ce50a0
│   tcp[884]           000002281cc4e430->00000228206c4a60
│  This means that a package has started a background task or event source that has not finished running. For precompilation to complete successfully, the event source needs to be closed explicitly. See the developer documentation on fixing precompilation hangs for more help.

Fwiw, the precompilation does always complete in reasonable time (maybe the connections are closed when gc:ed?) and I get pretty nice speedups from it, so this is mostly about not getting the warning.

I have tried putting HTTP.Connections.closeall() last in the precompile script but it did not help. However, this helped: foreach(vs -> foreach(close, vs), values(HTTP.Connections.OPENSSL_POOL[].keyedvalues)), but I don't feel comfortable having that code in there.

quinnj commented 6 months ago

Hmmmm, yeah, you're right that we don't actually close the connections when you call closeall, but that was also never meant to be a public user-facing API.

I think I would suggest that when you call HTTP.get (or whichever method you're using), you do something like: HTTP.get("https://www.google.com"; header=["Connection=>"close"]), which will ensure the connection is only used once and closed when done. I think that should avoid any lingering connections staying open that you don't want.

DrChainsaw commented 6 months ago

Thanks, but I couldn't get rid of the warnings.

Your example has a mismatched quote, so I tried the following permutations:

header=["Connection"=>"close"]
header=["Connection=>close"]
header=["Connection"=>close]
header=[HTTP.Connection=>close]
header=[HTTP.Connection=>"close"]

I am indeed using get. One of the two get calls adds a Dict as the query keyword argument. Could that impact whether the header argument is used? I added the header to both of them btw.

DrChainsaw commented 6 months ago

In case it is helpful, here is a MWE project:

module HttpPrecompile

using HTTP, PrecompileTools

@setup_workload begin
    @compile_workload begin
        @info "Start precompile"
        HTTP.get("https://www.google.com"; header=["Connection"=>"close"])
        #HTTP.Connections.closeall()
        #@show HTTP.Connections.OPENSSL_POOL[].keyedvalues
        #foreach(vs -> foreach(close, vs), values(HTTP.Connections.OPENSSL_POOL[].keyedvalues))
        @info "Precompile done"
    end
end

end # module HttpPrecompile
quinnj commented 6 months ago

Hmmm, this is a bit janky, but I think should work:

body = UInt8[]
resp = HTTP.open("GET", "https://www.github.com") do http
    while !eof(http)
        append!(body, readavailable(http)
    end
    close(HTTP.Connections.getrawstream(http))
end
DrChainsaw commented 6 months ago

Thanks, this prevents precompile from warning about dangling resources. Since I needed the body I also added resp.body = body so that the output looks the same as the normal get.

This still seems to rely on internals though (getrawstream and resp.body), right?

Any chance (a more convenient version of) it could be put in the public API?