search

dakrone / clj-http

An idiomatic clojure http client wrapping the apache client. Officially supported version.
http://clojars.org/clj-http
MIT License
1.78k stars 408 forks source link

Connections not released when using `:as :transit+json` on 3.10.x #564

Closed dmcgillen closed 3 years ago

dmcgillen commented 3 years ago

I'm using a socks proxy to debug some issues I've been having with an api call. The code I'm using is (I've replaced the url with my-url):

(def cm (clj-http.conn-mgr/make-socks-proxied-conn-manager "localhost" 10000))

(taoensso.timbre/with-log-level :debug
  (let [response (clj-http.client/get "my-url"
                                      {:accept :transit+json
                                       :as :transit+json
                                       :connection-manager cm})]))

Looking at the debug logs, I can see that connections are not being released after each call. After a couple of calls, it hangs because there are no free connections.

The logs at which it hangs are (waiting for a new connection):

20-09-23 09:31:32 Donovans-iMac.local DEBUG [org.apache.http.client.protocol.RequestAddCookies:123] - CookieSpec selected: default
20-09-23 09:31:32 Donovans-iMac.local DEBUG [org.apache.http.client.protocol.RequestAuthCache:77] - Auth cache not set in the context
20-09-23 09:31:32 Donovans-iMac.local DEBUG [org.apache.http.impl.conn.PoolingHttpClientConnectionManager:266] - Connection request: [route: {}->my-url][total kept alive: 0; route allocated: 2 of 2; total allocated: 2 of 20]

If I switch to use :as :json, I can see from the logs that after each response has been processed I get the following, showing that connections are released:

20-09-23 09:39:07 Donovans-iMac.local DEBUG [org.apache.http.impl.conn.PoolingHttpClientConnectionManager:342] - Connection [id: 7][route: {}->my-url] can be kept alive indefinitely
20-09-23 09:39:07 Donovans-iMac.local DEBUG [org.apache.http.impl.conn.DefaultManagedHttpClientConnection:88] - http-outgoing-7: set socket timeout to 0
20-09-23 09:39:07 Donovans-iMac.local DEBUG [org.apache.http.impl.conn.PoolingHttpClientConnectionManager:349] - Connection released: [id: 7][route: {}->my-url][total kept alive: 1; route allocated: 1 of 2; total allocated: 1 of 20]

and therefore everything works as expected. I do not get this when using transit+json.

I noticed the json responses were gzip encoded and the transit ones were not so have tried using :decompress-body false with json to see if the problem was to do with that, but it still works fine.

This behaviour occurs on 3.10.x versions, but not on 3.9.1.

Thanks!

rymndhng commented 3 years ago

@dmcgillen Nice find. I was able to reproduce this locally and I have a fix.

To provide some more context, in 3.10.x, there is an improvement to reduce copying https://github.com/dakrone/clj-http/pull/475. It seems we do not explicitly close the InputStream after reading an object using :transit.

Previously, this "worked" because the entire InputStream would be consumed into a byte-array and close the connection.

I've created #565 which should fix this in the next 3.10.x release.

dmcgillen commented 3 years ago

Thanks for the quick response and fix, @rymndhng, much appreciated.