Skip to content

Commit 37fc68c

Browse files
authored
Fix invalid thread local usage in #close. (#58)
- This could result in a memory leak.
1 parent b78b05d commit 37fc68c

File tree

3 files changed

+36
-3
lines changed

3 files changed

+36
-3
lines changed

lib/async/http/faraday/clients.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,10 @@ def with_proxied_client(proxy_endpoint, endpoint, &block)
177177
# This will close all clients associated with all threads.
178178
def close
179179
Thread.list.each do |thread|
180-
if clients = thread[@key]
181-
clients.close
180+
if clients = thread.thread_variable_get(@key)
181+
thread.thread_variable_set(@key, nil)
182182

183-
thread[@key] = nil
183+
clients.close
184184
end
185185
end
186186
end

releases.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Releases
22

3+
## Unreleased
4+
5+
- Fix memory leak in `Async::HTTP::Faraday::PerThreadPersistentClients` by ensuring that the `close` method is called on all clients when the adapter is closed.
6+
37
## v0.21.0
48

59
### Improved support for `timeout` and `read_timeout`.

test/async/http/faraday/clients.rb

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,33 @@
5757
end
5858
end
5959
end
60+
61+
with "#close" do
62+
it "closes all clients" do
63+
endpoint = Async::HTTP::Endpoint.parse("http://example.com")
64+
client = clients.make_client(endpoint)
65+
expect(client).to receive(:close)
66+
67+
clients.close
68+
end
69+
end
70+
end
71+
72+
describe Async::HTTP::Faraday::PerThreadPersistentClients do
73+
let(:clients) {subject.new}
74+
75+
with "#close" do
76+
it "closes all clients" do
77+
endpoint = Async::HTTP::Endpoint.parse("http://example.com")
78+
closed = false
79+
80+
clients.with_client(endpoint) do |client|
81+
expect(client).to receive(:close, &proc{closed = true})
82+
end
83+
84+
expect(closed).to be == false
85+
clients.close
86+
expect(closed).to be == true
87+
end
88+
end
6089
end

0 commit comments

Comments
 (0)