Skip to content

Commit 468a55b

Browse files
committed
Merge pull request rails#20866 from jdantonio/countdown-latch
Replace `ActiveSupport::Concurrency::Latch` with `Concurrent::CountDownLatch` from concurrent-ruby.
2 parents 7645a5f + 284a9ba commit 468a55b

File tree

11 files changed

+70
-58
lines changed

11 files changed

+70
-58
lines changed

Gemfile.lock

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ PATH
6464
actionpack (5.0.0.alpha)
6565
actionview (= 5.0.0.alpha)
6666
activesupport (= 5.0.0.alpha)
67+
concurrent-ruby (~> 0.9.0)
6768
rack (~> 1.6)
6869
rack-test (~> 0.6.3)
6970
rails-dom-testing (~> 1.0, >= 1.0.5)
@@ -85,6 +86,7 @@ PATH
8586
activesupport (= 5.0.0.alpha)
8687
arel (= 7.0.0.alpha)
8788
activesupport (5.0.0.alpha)
89+
concurrent-ruby (~> 0.9.0)
8890
i18n (~> 0.7)
8991
json (~> 1.7, >= 1.7.7)
9092
minitest (~> 5.1)
@@ -135,6 +137,7 @@ GEM
135137
execjs
136138
coffee-script-source (1.9.0)
137139
columnize (0.9.0)
140+
concurrent-ruby (0.9.0)
138141
connection_pool (2.1.1)
139142
dalli (2.7.2)
140143
dante (0.1.5)

actionpack/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
* Replaced `ActiveSupport::Concurrency::Latch` with `Concurrent::CountDownLatch`
2+
from the concurrent-ruby gem.
3+
4+
*Jerry D'Antonio*
5+
16
* Add ability to filter parameters based on parent keys.
27

38
# matches {credit_card: {code: "xxxx"}}

actionpack/actionpack.gemspec

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Gem::Specification.new do |s|
2626
s.add_dependency 'rails-html-sanitizer', '~> 1.0', '>= 1.0.2'
2727
s.add_dependency 'rails-dom-testing', '~> 1.0', '>= 1.0.5'
2828
s.add_dependency 'actionview', version
29+
s.add_dependency 'concurrent-ruby', '~> 0.9.0'
2930

3031
s.add_development_dependency 'activemodel', version
3132
end

actionpack/test/controller/live_stream_test.rb

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
require 'abstract_unit'
2-
require 'active_support/concurrency/latch'
2+
require 'concurrent/atomics'
33
Thread.abort_on_exception = true
44

55
module ActionController
@@ -145,7 +145,7 @@ def blocking_stream
145145
response.headers['Content-Type'] = 'text/event-stream'
146146
%w{ hello world }.each do |word|
147147
response.stream.write word
148-
latch.await
148+
latch.wait
149149
end
150150
response.stream.close
151151
end
@@ -212,7 +212,7 @@ def overfill_buffer_and_die
212212
# .. plus one more, because the #each frees up a slot:
213213
response.stream.write '.'
214214

215-
latch.release
215+
latch.count_down
216216

217217
# This write will block, and eventually raise
218218
response.stream.write 'x'
@@ -233,7 +233,7 @@ def ignore_client_disconnect
233233
end
234234

235235
logger.info 'Work complete'
236-
latch.release
236+
latch.count_down
237237
end
238238
end
239239

@@ -278,7 +278,7 @@ def test_write_to_stream
278278
def test_async_stream
279279
rubinius_skip "https://github.com/rubinius/rubinius/issues/2934"
280280

281-
@controller.latch = ActiveSupport::Concurrency::Latch.new
281+
@controller.latch = Concurrent::CountDownLatch.new
282282
parts = ['hello', 'world']
283283

284284
@controller.request = @request
@@ -289,8 +289,8 @@ def test_async_stream
289289
resp.stream.each do |part|
290290
assert_equal parts.shift, part
291291
ol = @controller.latch
292-
@controller.latch = ActiveSupport::Concurrency::Latch.new
293-
ol.release
292+
@controller.latch = Concurrent::CountDownLatch.new
293+
ol.count_down
294294
end
295295
}
296296

@@ -300,23 +300,23 @@ def test_async_stream
300300
end
301301

302302
def test_abort_with_full_buffer
303-
@controller.latch = ActiveSupport::Concurrency::Latch.new
303+
@controller.latch = Concurrent::CountDownLatch.new
304304

305305
@request.parameters[:format] = 'plain'
306306
@controller.request = @request
307307
@controller.response = @response
308308

309-
got_error = ActiveSupport::Concurrency::Latch.new
309+
got_error = Concurrent::CountDownLatch.new
310310
@response.stream.on_error do
311311
ActionController::Base.logger.warn 'Error while streaming'
312-
got_error.release
312+
got_error.count_down
313313
end
314314

315315
t = Thread.new(@response) { |resp|
316316
resp.await_commit
317317
_, _, body = resp.to_a
318318
body.each do |part|
319-
@controller.latch.await
319+
@controller.latch.wait
320320
body.close
321321
break
322322
end
@@ -325,13 +325,13 @@ def test_abort_with_full_buffer
325325
capture_log_output do |output|
326326
@controller.process :overfill_buffer_and_die
327327
t.join
328-
got_error.await
328+
got_error.wait
329329
assert_match 'Error while streaming', output.rewind && output.read
330330
end
331331
end
332332

333333
def test_ignore_client_disconnect
334-
@controller.latch = ActiveSupport::Concurrency::Latch.new
334+
@controller.latch = Concurrent::CountDownLatch.new
335335

336336
@controller.request = @request
337337
@controller.response = @response
@@ -349,7 +349,7 @@ def test_ignore_client_disconnect
349349
@controller.process :ignore_client_disconnect
350350
t.join
351351
Timeout.timeout(3) do
352-
@controller.latch.await
352+
@controller.latch.wait
353353
end
354354
assert_match 'Work complete', output.rewind && output.read
355355
end

actionpack/test/dispatch/live_response_test.rb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
require 'abstract_unit'
2-
require 'active_support/concurrency/latch'
2+
require 'concurrent/atomics'
33

44
module ActionController
55
module Live
@@ -27,18 +27,18 @@ def self.default_headers
2727
end
2828

2929
def test_parallel
30-
latch = ActiveSupport::Concurrency::Latch.new
30+
latch = Concurrent::CountDownLatch.new
3131

3232
t = Thread.new {
3333
@response.stream.write 'foo'
34-
latch.await
34+
latch.wait
3535
@response.stream.close
3636
}
3737

3838
@response.await_commit
3939
@response.each do |part|
4040
assert_equal 'foo', part
41-
latch.release
41+
latch.count_down
4242
end
4343
assert t.join
4444
end
@@ -62,15 +62,15 @@ def test_content_length_is_removed
6262

6363
def test_headers_cannot_be_written_after_webserver_reads
6464
@response.stream.write 'omg'
65-
latch = ActiveSupport::Concurrency::Latch.new
65+
latch = Concurrent::CountDownLatch.new
6666

6767
t = Thread.new {
6868
@response.stream.each do |chunk|
69-
latch.release
69+
latch.count_down
7070
end
7171
}
7272

73-
latch.await
73+
latch.wait
7474
assert @response.headers.frozen?
7575
e = assert_raises(ActionDispatch::IllegalStateError) do
7676
@response.headers['Content-Length'] = "zomg"

activerecord/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
* Replaced `ActiveSupport::Concurrency::Latch` with `Concurrent::CountDownLatch`
2+
from the concurrent-ruby gem.
3+
4+
*Jerry D'Antonio*
5+
16
* Fix through associations using scopes having the scope merged multiple
27
times.
38

activerecord/test/cases/base_test.rb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# -*- coding: utf-8 -*-
22

33
require "cases/helper"
4-
require 'active_support/concurrency/latch'
54
require 'models/post'
65
require 'models/author'
76
require 'models/topic'
@@ -29,6 +28,7 @@
2928
require 'models/car'
3029
require 'models/bulb'
3130
require 'rexml/document'
31+
require 'concurrent/atomics'
3232

3333
class FirstAbstractClass < ActiveRecord::Base
3434
self.abstract_class = true
@@ -1506,20 +1506,20 @@ def test_default_values_are_deeply_dupped
15061506
orig_handler = klass.connection_handler
15071507
new_handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new
15081508
after_handler = nil
1509-
latch1 = ActiveSupport::Concurrency::Latch.new
1510-
latch2 = ActiveSupport::Concurrency::Latch.new
1509+
latch1 = Concurrent::CountDownLatch.new
1510+
latch2 = Concurrent::CountDownLatch.new
15111511

15121512
t = Thread.new do
15131513
klass.connection_handler = new_handler
1514-
latch1.release
1515-
latch2.await
1514+
latch1.count_down
1515+
latch2.wait
15161516
after_handler = klass.connection_handler
15171517
end
15181518

1519-
latch1.await
1519+
latch1.wait
15201520

15211521
klass.connection_handler = orig_handler
1522-
latch2.release
1522+
latch2.count_down
15231523
t.join
15241524

15251525
assert_equal after_handler, new_handler

activerecord/test/cases/connection_pool_test.rb

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
require "cases/helper"
2-
require 'active_support/concurrency/latch'
2+
require 'concurrent/atomics'
33

44
module ActiveRecord
55
module ConnectionAdapters
@@ -133,15 +133,15 @@ def test_reap_and_active
133133
end
134134

135135
def test_reap_inactive
136-
ready = ActiveSupport::Concurrency::Latch.new
136+
ready = Concurrent::CountDownLatch.new
137137
@pool.checkout
138138
child = Thread.new do
139139
@pool.checkout
140140
@pool.checkout
141-
ready.release
141+
ready.count_down
142142
Thread.stop
143143
end
144-
ready.await
144+
ready.wait
145145

146146
assert_equal 3, active_connections(@pool).size
147147

@@ -360,13 +360,13 @@ def test_pool_sets_connection_schema_cache
360360
def test_concurrent_connection_establishment
361361
assert_operator @pool.connections.size, :<=, 1
362362

363-
all_threads_in_new_connection = ActiveSupport::Concurrency::Latch.new(@pool.size - @pool.connections.size)
364-
all_go = ActiveSupport::Concurrency::Latch.new
363+
all_threads_in_new_connection = Concurrent::CountDownLatch.new(@pool.size - @pool.connections.size)
364+
all_go = Concurrent::CountDownLatch.new
365365

366366
@pool.singleton_class.class_eval do
367367
define_method(:new_connection) do
368-
all_threads_in_new_connection.release
369-
all_go.await
368+
all_threads_in_new_connection.count_down
369+
all_go.wait
370370
super()
371371
end
372372
end
@@ -381,14 +381,14 @@ def test_concurrent_connection_establishment
381381
# the kernel of the whole test is here, everything else is just scaffolding,
382382
# this latch will not be released unless conn. pool allows for concurrent
383383
# connection creation
384-
all_threads_in_new_connection.await
384+
all_threads_in_new_connection.wait
385385
end
386386
rescue Timeout::Error
387387
flunk 'pool unable to establish connections concurrently or implementation has ' <<
388388
'changed, this test then needs to patch a different :new_connection method'
389389
ensure
390390
# clean up the threads
391-
all_go.release
391+
all_go.count_down
392392
connecting_threads.map(&:join)
393393
end
394394
end
@@ -441,11 +441,11 @@ def test_disconnect_and_clear_reloadable_connections_are_able_to_preempt_other_w
441441
with_single_connection_pool do |pool|
442442
[:disconnect, :disconnect!, :clear_reloadable_connections, :clear_reloadable_connections!].each do |group_action_method|
443443
conn = pool.connection # drain the only available connection
444-
second_thread_done = ActiveSupport::Concurrency::Latch.new
444+
second_thread_done = Concurrent::CountDownLatch.new
445445

446446
# create a first_thread and let it get into the FIFO queue first
447447
first_thread = Thread.new do
448-
pool.with_connection { second_thread_done.await }
448+
pool.with_connection { second_thread_done.wait }
449449
end
450450

451451
# wait for first_thread to get in queue
@@ -456,7 +456,7 @@ def test_disconnect_and_clear_reloadable_connections_are_able_to_preempt_other_w
456456
# first_thread when a connection is made available
457457
second_thread = Thread.new do
458458
pool.send(group_action_method)
459-
second_thread_done.release
459+
second_thread_done.count_down
460460
end
461461

462462
# wait for second_thread to get in queue
@@ -471,7 +471,7 @@ def test_disconnect_and_clear_reloadable_connections_are_able_to_preempt_other_w
471471
failed = true unless second_thread.join(2)
472472

473473
#--- post test clean up start
474-
second_thread_done.release if failed
474+
second_thread_done.count_down if failed
475475

476476
# after `pool.disconnect()` the first thread will be left stuck in queue, no need to wait for
477477
# it to timeout with ConnectionTimeoutError

activesupport/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
* Removed `ActiveSupport::Concurrency::Latch`, superseded by `Concurrent::CountDownLatch`
2+
from the concurrent-ruby gem.
3+
4+
*Jerry D'Antonio*
5+
16
* Fix not calling `#default` on `HashWithIndifferentAccess#to_hash` when only
27
`default_proc` is set, which could raise.
38

activesupport/activesupport.gemspec

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,5 @@ Gem::Specification.new do |s|
2525
s.add_dependency 'tzinfo', '~> 1.1'
2626
s.add_dependency 'minitest', '~> 5.1'
2727
s.add_dependency 'thread_safe','~> 0.3', '>= 0.3.4'
28+
s.add_dependency 'concurrent-ruby', '~> 0.9.0'
2829
end

0 commit comments

Comments
 (0)