Skip to content

Commit b1b8706

Browse files
wtnclaude
andcommitted
Handle retire/release race when resource close yields.
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 42f2142 commit b1b8706

3 files changed

Lines changed: 86 additions & 6 deletions

File tree

lib/async/pool/controller.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ def prune(retain = 0)
234234
def retire(resource)
235235
Console.debug(self){"Retire #{resource}"}
236236

237-
@resources.delete(resource)
237+
return false unless @resources.delete(resource)
238238

239239
resource.close
240240

@@ -286,7 +286,11 @@ def reuse(resource)
286286

287287
usage = @resources[resource]
288288

289-
if usage.nil? || usage.zero?
289+
if usage.nil?
290+
return false
291+
end
292+
293+
if usage.zero?
290294
raise "Trying to reuse unacquired resource: #{resource}!"
291295
end
292296

test/async/pool/controller.rb

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,10 @@
2222
expect(pool).to be(:empty?)
2323
end
2424

25-
it "raises an error when releasing an unacquired resource" do
25+
it "silently handles releasing an unacquired resource" do
2626
resource = Async::Pool::Resource.new
2727

28-
expect do
29-
pool.release(resource)
30-
end.to raise_exception(RuntimeError, message: be =~ /unacquired resource/)
28+
pool.release(resource)
3129
end
3230

3331
with "#as_json" do

test/async/pool/retire_race.rb

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# frozen_string_literal: true
2+
3+
# Verifies that retire + release on the same resource does not raise
4+
# "Trying to reuse unacquired resource".
5+
#
6+
# In practice this happens with async-http when an HTTP/2 connection is reset:
7+
# the background reader retires the connection, but retire's resource.close
8+
# yields (e.g. sending GOAWAY), allowing another fiber to call release while
9+
# the resource is deleted from @resources but still reports reusable? = true.
10+
11+
require "async/pool/controller"
12+
require "async/pool/resource"
13+
require "sus/fixtures/async/reactor_context"
14+
15+
# A resource whose close yields (simulating async IO like sending GOAWAY),
16+
# but whose reusable? check is synchronous (no yield).
17+
class SlowCloseResource < Async::Pool::Resource
18+
def close
19+
Async::Task.current.yield
20+
super
21+
end
22+
end
23+
24+
describe Async::Pool::Controller do
25+
include Sus::Fixtures::Async::ReactorContext
26+
27+
with "retire/release race on slow-close resource" do
28+
let(:pool) {subject.new(SlowCloseResource)}
29+
30+
it "gracefully handles release after retire begins closing" do
31+
resource = pool.acquire
32+
33+
# Start retire in a child task. It runs synchronously up to the
34+
# yield inside SlowCloseResource#close, then pauses. At that point
35+
# @resources[resource] has been deleted but resource.close hasn't
36+
# finished, so reusable? still returns true.
37+
retire_task = Async do
38+
pool.retire(resource)
39+
end
40+
41+
# No yield needed — the child already ran to its yield point.
42+
# Verify the race window exists:
43+
expect(resource).to be(:reusable?)
44+
expect(pool.resources).not.to be(:key?, resource)
45+
46+
# The client's error handler now tries to release the same resource.
47+
# This should not raise — retire already claimed ownership.
48+
pool.release(resource)
49+
50+
retire_task.wait
51+
end
52+
53+
it "gracefully handles multiplexed release after retire begins closing" do
54+
constructor = lambda{SlowCloseResource.new(128)}
55+
pool = subject.new(constructor)
56+
57+
# Two streams on the same HTTP/2 connection:
58+
r1 = pool.acquire
59+
r2 = pool.acquire
60+
expect(r1).to be_equal(r2)
61+
62+
# Background reader retires (yields during close):
63+
retire_task = Async do
64+
pool.retire(r1)
65+
end
66+
67+
# The race window is open: resource deleted from pool but not
68+
# yet closed. First stream's error handler hits the race:
69+
expect(r1).to be(:reusable?)
70+
expect(pool.resources).not.to be(:key?, r1)
71+
72+
# Should not raise:
73+
pool.release(r1)
74+
75+
retire_task.wait
76+
end
77+
end
78+
end

0 commit comments

Comments
 (0)