From b0e2216de2ab6229c18dbba98018e921b047c226 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Wed, 29 Mar 2017 22:35:25 +0100 Subject: [PATCH 1/2] Fix failing testcase when using SynchronizedMapBackend The SynchronizedMapBackend, by definition, uses a lock around every operation, and thus could never pass a test that checks that updates and reads can happen concurrently. --- spec/concurrent/map_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/concurrent/map_spec.rb b/spec/concurrent/map_spec.rb index 99f1fa9b4..8c75fa24e 100644 --- a/spec/concurrent/map_spec.rb +++ b/spec/concurrent/map_spec.rb @@ -255,6 +255,10 @@ module Concurrent end it 'updates dont block reads' do + if defined?(Concurrent::Collection::SynchronizedMapBackend) && described_class <= Concurrent::Collection::SynchronizedMapBackend + skip("Test does not apply to #{Concurrent::Collection::SynchronizedMapBackend}") + end + getters_count = 20 key_klass = Concurrent::ThreadSafe::Test::HashCollisionKey keys = [key_klass.new(1, 100), From 0bdc3c01db99f54b177d5e58d5b58b8bb064e8c8 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Wed, 29 Mar 2017 22:36:54 +0100 Subject: [PATCH 2/2] Fix Map#each and #each_pair not returning enumerator outside of MRI When no block was passed to Map#each_pair or its alias Map#each, the NonConcurrentMapBackend would return an enumerator, which would allow enumerable methods to be used on a Map. But the alternate Map backends, JRubyMapBackend and AtomicReferenceMapBackend did not implement the same logic. As a fix, let's move the logic down to the shared Map class, so all implementations can use it. Fixes #643 --- .../collection/map/non_concurrent_map_backend.rb | 1 - lib/concurrent/map.rb | 5 +++++ spec/concurrent/collection_each_shared.rb | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/concurrent/collection/map/non_concurrent_map_backend.rb b/lib/concurrent/collection/map/non_concurrent_map_backend.rb index ba86d7c0f..1c9aa8984 100644 --- a/lib/concurrent/collection/map/non_concurrent_map_backend.rb +++ b/lib/concurrent/collection/map/non_concurrent_map_backend.rb @@ -95,7 +95,6 @@ def clear end def each_pair - return enum_for :each_pair unless block_given? dupped_backend.each_pair do |k, v| yield k, v end diff --git a/lib/concurrent/map.rb b/lib/concurrent/map.rb index 814deb008..f3aa55d53 100644 --- a/lib/concurrent/map.rb +++ b/lib/concurrent/map.rb @@ -171,6 +171,11 @@ def each_value each_pair {|k, v| yield v} end unless method_defined?(:each_value) + def each_pair + return enum_for :each_pair unless block_given? + super + end + alias_method :each, :each_pair unless method_defined?(:each) def key(value) diff --git a/spec/concurrent/collection_each_shared.rb b/spec/concurrent/collection_each_shared.rb index 55de1319c..c4c970707 100644 --- a/spec/concurrent/collection_each_shared.rb +++ b/spec/concurrent/collection_each_shared.rb @@ -42,4 +42,20 @@ end end end + + context 'when no block is given' do + it 'returns an enumerator' do + @cache[:a] = 1 + @cache[:b] = 2 + + expect(@cache.send(method)).to be_a Enumerator + end + + it 'returns an object which is enumerable' do + @cache[:a] = 1 + @cache[:b] = 2 + + expect(@cache.send(method).to_a).to contain_exactly([:a, 1], [:b, 2]) + end + end end