diff options
Diffstat (limited to 'activesupport')
16 files changed, 399 insertions, 71 deletions
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 21c79949ca..bd333da081 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,23 @@ +* Fix regression in `Hash#dig` for HashWithIndifferentAccess. + *Jon Moss* + +## Rails 5.0.0.beta2 (February 01, 2016) ## + +* Change number_to_currency behavior for checking negativity. + + Used `to_f.negative` instead of using `to_f.phase` for checking negativity + of a number in number_to_currency helper. + This change works same for all cases except when number is "-0.0". + + -0.0.to_f.negative? => false + -0.0.to_f.phase? => 3.14 + + This change reverts changes from https://github.com/rails/rails/pull/6512. + But it should be acceptable as we could not find any currency which + supports negative zeros. + + *Prathamesh Sonpatki*, *Rafael Mendonça França* + * Match `HashWithIndifferentAccess#default`'s behaviour with `Hash#default`. *David Cornu* @@ -27,44 +47,44 @@ Here's an example of a simple event tracking system where the object being tracked needs not pass a creator that it doesn't need itself along: - module Current - thread_mattr_accessor :account - thread_mattr_accessor :user + module Current + thread_mattr_accessor :account + thread_mattr_accessor :user - def self.reset() self.account = self.user = nil end - end + def self.reset() self.account = self.user = nil end + end - class ApplicationController < ActionController::Base - before_action :set_current - after_action { Current.reset } + class ApplicationController < ActionController::Base + before_action :set_current + after_action { Current.reset } - private - def set_current - Current.account = Account.find(params[:account_id]) - Current.user = Current.account.users.find(params[:user_id]) + private + def set_current + Current.account = Account.find(params[:account_id]) + Current.user = Current.account.users.find(params[:user_id]) + end end - end - class MessagesController < ApplicationController - def create - @message = Message.create!(message_params) - end - end + class MessagesController < ApplicationController + def create + @message = Message.create!(message_params) + end + end - class Message < ApplicationRecord - has_many :events - after_create :track_created + class Message < ApplicationRecord + has_many :events + after_create :track_created - private - def track_created - events.create! origin: self, action: :create + private + def track_created + events.create! origin: self, action: :create + end end - end - class Event < ApplicationRecord - belongs_to :creator, class_name: 'User' - before_validation { self.creator ||= Current.user } - end + class Event < ApplicationRecord + belongs_to :creator, class_name: 'User' + before_validation { self.creator ||= Current.user } + end *DHH* diff --git a/activesupport/Rakefile b/activesupport/Rakefile index 81c242d4b1..33ee62aa1b 100644 --- a/activesupport/Rakefile +++ b/activesupport/Rakefile @@ -1,6 +1,10 @@ require 'rake/testtask' task :default => :test + +task :package +task "package:clean" + Rake::TestTask.new do |t| t.libs << 'test' t.pattern = 'test/**/*_test.rb' diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index 32e28c0212..68a80701ed 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -21,9 +21,7 @@ Gem::Specification.new do |s| s.rdoc_options.concat ['--encoding', 'UTF-8'] s.add_dependency 'i18n', '~> 0.7' - s.add_dependency 'json', '~> 1.7', '>= 1.7.7' s.add_dependency 'tzinfo', '~> 1.1' s.add_dependency 'minitest', '~> 5.1' s.add_dependency 'concurrent-ruby', '~> 1.0' - s.add_dependency 'method_source' end diff --git a/activesupport/lib/active_support/concurrency/share_lock.rb b/activesupport/lib/active_support/concurrency/share_lock.rb index ca48164c54..54244317e4 100644 --- a/activesupport/lib/active_support/concurrency/share_lock.rb +++ b/activesupport/lib/active_support/concurrency/share_lock.rb @@ -6,12 +6,6 @@ module ActiveSupport # A share/exclusive lock, otherwise known as a read/write lock. # # https://en.wikipedia.org/wiki/Readers%E2%80%93writer_lock - #-- - # Note that a pending Exclusive lock attempt does not block incoming - # Share requests (i.e., we are "read-preferring"). That seems - # consistent with the behavior of "loose" upgrades, but may be the - # wrong choice otherwise: it nominally reduces the possibility of - # deadlock by risking starvation instead. class ShareLock include MonitorMixin @@ -48,17 +42,11 @@ module ActiveSupport def start_exclusive(purpose: nil, compatible: [], no_wait: false) synchronize do unless @exclusive_thread == Thread.current - if busy?(purpose) + if busy_for_exclusive?(purpose) return false if no_wait - loose_shares = @sharing.delete(Thread.current) - @waiting[Thread.current] = compatible if loose_shares - - begin - @cv.wait_while { busy?(purpose) } - ensure - @waiting.delete Thread.current - @sharing[Thread.current] = loose_shares if loose_shares + yield_shares(purpose: purpose, compatible: compatible, block_share: true) do + @cv.wait_while { busy_for_exclusive?(purpose) } end end @exclusive_thread = Thread.current @@ -71,13 +59,19 @@ module ActiveSupport # Relinquish the exclusive lock. Must only be called by the thread # that called start_exclusive (and currently holds the lock). - def stop_exclusive + def stop_exclusive(compatible: []) synchronize do raise "invalid unlock" if @exclusive_thread != Thread.current @exclusive_depth -= 1 if @exclusive_depth == 0 @exclusive_thread = nil + + if eligible_waiters?(compatible) + yield_shares(compatible: compatible, block_share: true) do + @cv.wait_while { @exclusive_thread || eligible_waiters?(compatible) } + end + end @cv.broadcast end end @@ -85,8 +79,16 @@ module ActiveSupport def start_sharing synchronize do - if @exclusive_thread && @exclusive_thread != Thread.current + if @sharing[Thread.current] > 0 || @exclusive_thread == Thread.current + # We already hold a lock; nothing to wait for + elsif @waiting[Thread.current] + # We're nested inside a +yield_shares+ call: we'll resume as + # soon as there isn't an exclusive lock in our way @cv.wait_while { @exclusive_thread } + else + # This is an initial / outermost share call: any outstanding + # requests for an exclusive lock get to go first + @cv.wait_while { busy_for_sharing?(false) } end @sharing[Thread.current] += 1 end @@ -109,12 +111,12 @@ module ActiveSupport # the block. # # See +start_exclusive+ for other options. - def exclusive(purpose: nil, compatible: [], no_wait: false) + def exclusive(purpose: nil, compatible: [], after_compatible: [], no_wait: false) if start_exclusive(purpose: purpose, compatible: compatible, no_wait: no_wait) begin yield ensure - stop_exclusive + stop_exclusive(compatible: after_compatible) end end end @@ -129,14 +131,56 @@ module ActiveSupport end end + # Temporarily give up all held Share locks while executing the + # supplied block, allowing any +compatible+ exclusive lock request + # to proceed. + def yield_shares(purpose: nil, compatible: [], block_share: false) + loose_shares = previous_wait = nil + synchronize do + if loose_shares = @sharing.delete(Thread.current) + if previous_wait = @waiting[Thread.current] + purpose = nil unless purpose == previous_wait[0] + compatible &= previous_wait[1] + end + compatible |= [false] unless block_share + @waiting[Thread.current] = [purpose, compatible] + + @cv.broadcast + end + end + + begin + yield + ensure + synchronize do + @cv.wait_while { @exclusive_thread && @exclusive_thread != Thread.current } + + if previous_wait + @waiting[Thread.current] = previous_wait + else + @waiting.delete Thread.current + end + @sharing[Thread.current] = loose_shares if loose_shares + end + end + end + private # Must be called within synchronize - def busy?(purpose) - (@exclusive_thread && @exclusive_thread != Thread.current) || - @waiting.any? { |k, v| k != Thread.current && !v.include?(purpose) } || + def busy_for_exclusive?(purpose) + busy_for_sharing?(purpose) || @sharing.size > (@sharing[Thread.current] > 0 ? 1 : 0) end + + def busy_for_sharing?(purpose) + (@exclusive_thread && @exclusive_thread != Thread.current) || + @waiting.any? { |t, (_, c)| t != Thread.current && !c.include?(purpose) } + end + + def eligible_waiters?(compatible) + @waiting.any? { |t, (p, _)| compatible.include?(p) && @waiting.all? { |t2, (_, c2)| t == t2 || c2.include?(p) } } + end end end end diff --git a/activesupport/lib/active_support/core_ext/kernel/concern.rb b/activesupport/lib/active_support/core_ext/kernel/concern.rb index bf72caa058..18bcc01fa4 100644 --- a/activesupport/lib/active_support/core_ext/kernel/concern.rb +++ b/activesupport/lib/active_support/core_ext/kernel/concern.rb @@ -1,6 +1,8 @@ require 'active_support/core_ext/module/concerning' module Kernel + module_function + # A shortcut to define a toplevel concern, not within a module. # # See Module::Concerning for more. diff --git a/activesupport/lib/active_support/core_ext/kernel/reporting.rb b/activesupport/lib/active_support/core_ext/kernel/reporting.rb index 8afc258df8..d0197af95f 100644 --- a/activesupport/lib/active_support/core_ext/kernel/reporting.rb +++ b/activesupport/lib/active_support/core_ext/kernel/reporting.rb @@ -1,4 +1,6 @@ module Kernel + module_function + # Sets $VERBOSE to nil for the duration of the block and back to its original # value afterwards. # diff --git a/activesupport/lib/active_support/core_ext/module/deprecation.rb b/activesupport/lib/active_support/core_ext/module/deprecation.rb index 56d670fbe8..f3f2e7f5fc 100644 --- a/activesupport/lib/active_support/core_ext/module/deprecation.rb +++ b/activesupport/lib/active_support/core_ext/module/deprecation.rb @@ -13,8 +13,8 @@ class Module # # class MyLib::Deprecator # def deprecation_warning(deprecated_method_name, message, caller_backtrace = nil) - # message = "#{deprecated_method_name} is deprecated and will be removed from MyLibrary | #{message}" - # Kernel.warn message + # message = "#{deprecated_method_name} is deprecated and will be removed from MyLibrary | #{message}" + # Kernel.warn message # end # end def deprecate(*method_names) diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb index 6251f34daf..43b9fd4bf7 100644 --- a/activesupport/lib/active_support/core_ext/string/output_safety.rb +++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb @@ -171,7 +171,7 @@ module ActiveSupport #:nodoc: original_concat(value) end - def initialize(*) + def initialize(str = '') @html_safe = true super end diff --git a/activesupport/lib/active_support/dependencies/interlock.rb b/activesupport/lib/active_support/dependencies/interlock.rb index fbeb904684..47bcecbb35 100644 --- a/activesupport/lib/active_support/dependencies/interlock.rb +++ b/activesupport/lib/active_support/dependencies/interlock.rb @@ -8,13 +8,13 @@ module ActiveSupport #:nodoc: end def loading - @lock.exclusive(purpose: :load, compatible: [:load]) do + @lock.exclusive(purpose: :load, compatible: [:load], after_compatible: [:load]) do yield end end def unloading - @lock.exclusive(purpose: :unload, compatible: [:load, :unload]) do + @lock.exclusive(purpose: :unload, compatible: [:load, :unload], after_compatible: [:load, :unload]) do yield end end @@ -24,7 +24,7 @@ module ActiveSupport #:nodoc: # concurrent activity, return immediately (without executing the # block) instead of waiting. def attempt_unloading - @lock.exclusive(purpose: :unload, compatible: [:load, :unload], no_wait: true) do + @lock.exclusive(purpose: :unload, compatible: [:load, :unload], after_compatible: [:load, :unload], no_wait: true) do yield end end @@ -42,6 +42,12 @@ module ActiveSupport #:nodoc: yield end end + + def permit_concurrent_loads + @lock.yield_shares(compatible: [:load]) do + yield + end + end end end end diff --git a/activesupport/lib/active_support/deprecation/proxy_wrappers.rb b/activesupport/lib/active_support/deprecation/proxy_wrappers.rb index 6f0ad445fc..0cb2d4d22e 100644 --- a/activesupport/lib/active_support/deprecation/proxy_wrappers.rb +++ b/activesupport/lib/active_support/deprecation/proxy_wrappers.rb @@ -80,7 +80,7 @@ module ActiveSupport # example.old_request.to_s # # => DEPRECATION WARNING: @request is deprecated! Call request.to_s instead of # @request.to_s - # (Bactrace information…) + # (Backtrace information…) # "special_request" # # example.request.to_s @@ -118,7 +118,7 @@ module ActiveSupport # # PLANETS.map { |planet| planet.capitalize } # # => DEPRECATION WARNING: PLANETS is deprecated! Use PLANETS_POST_2006 instead. - # (Bactrace information…) + # (Backtrace information…) # ["Mercury", "Venus", "Earth", "Mars", "Jupiter", "Saturn", "Uranus", "Neptune"] class DeprecatedConstantProxy < DeprecationProxy def initialize(old_const, new_const, deprecator = ActiveSupport::Deprecation.instance) diff --git a/activesupport/lib/active_support/gem_version.rb b/activesupport/lib/active_support/gem_version.rb index 3104d0e00f..fc08273b6d 100644 --- a/activesupport/lib/active_support/gem_version.rb +++ b/activesupport/lib/active_support/gem_version.rb @@ -8,7 +8,7 @@ module ActiveSupport MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta1.1" + PRE = "beta2" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index b878f31e75..03770a197c 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -69,9 +69,13 @@ module ActiveSupport end def default(*args) - key = args.first - args[0] = key.to_s if key.is_a?(Symbol) - super(*args) + arg_key = args.first + + if include?(key = convert_key(arg_key)) + self[key] + else + super + end end def self.new_from_hash_copying_default(hash) diff --git a/activesupport/lib/active_support/number_helper/number_to_currency_converter.rb b/activesupport/lib/active_support/number_helper/number_to_currency_converter.rb index 7986eb50f0..57f40f33bf 100644 --- a/activesupport/lib/active_support/number_helper/number_to_currency_converter.rb +++ b/activesupport/lib/active_support/number_helper/number_to_currency_converter.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/numeric/inquiry' + module ActiveSupport module NumberHelper class NumberToCurrencyConverter < NumberConverter # :nodoc: @@ -7,7 +9,7 @@ module ActiveSupport number = self.number.to_s.strip format = options[:format] - if is_negative?(number) + if number.to_f.negative? format = options[:negative_format] number = absolute_value(number) end @@ -18,10 +20,6 @@ module ActiveSupport private - def is_negative?(number) - number.to_f.phase != 0 - end - def absolute_value(number) number.respond_to?(:abs) ? number.abs : number.sub(/\A-/, '') end diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 1b66f784e4..be8583e704 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -702,6 +702,12 @@ class HashExtTest < ActiveSupport::TestCase assert_equal h.class, h.dup.class end + def test_nested_dig_indifferent_access + skip if RUBY_VERSION < "2.3.0" + data = {"this" => {"views" => 1234}}.with_indifferent_access + assert_equal 1234, data.dig(:this, :views) + end + def test_assert_valid_keys assert_nothing_raised do { :failure => "stuff", :funny => "business" }.assert_valid_keys([ :failure, :funny ]) diff --git a/activesupport/test/number_helper_test.rb b/activesupport/test/number_helper_test.rb index b3464462c8..6696111476 100644 --- a/activesupport/test/number_helper_test.rb +++ b/activesupport/test/number_helper_test.rb @@ -74,7 +74,6 @@ module ActiveSupport assert_equal("1,234,567,890.50 Kč", number_helper.number_to_currency("1234567890.50", {:unit => "Kč", :format => "%n %u"})) assert_equal("1,234,567,890.50 - Kč", number_helper.number_to_currency("-1234567890.50", {:unit => "Kč", :format => "%n %u", :negative_format => "%n - %u"})) assert_equal("0.00", number_helper.number_to_currency(+0.0, {:unit => "", :negative_format => "(%n)"})) - assert_equal("(0.00)", number_helper.number_to_currency(-0.0, {:unit => "", :negative_format => "(%n)"})) end end diff --git a/activesupport/test/share_lock_test.rb b/activesupport/test/share_lock_test.rb index 465a657308..acefa185a8 100644 --- a/activesupport/test/share_lock_test.rb +++ b/activesupport/test/share_lock_test.rb @@ -114,14 +114,17 @@ class ShareLockTest < ActiveSupport::TestCase [true, false].each do |use_upgrading| with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch| begin + together = Concurrent::CyclicBarrier.new(2) conflicting_exclusive_threads = [ Thread.new do @lock.send(use_upgrading ? :sharing : :tap) do + together.wait @lock.exclusive(purpose: :red, compatible: [:green, :purple]) {} end end, Thread.new do @lock.send(use_upgrading ? :sharing : :tap) do + together.wait @lock.exclusive(purpose: :blue, compatible: [:green]) {} end end @@ -183,11 +186,14 @@ class ShareLockTest < ActiveSupport::TestCase load_params = [:load, [:load]] unload_params = [:unload, [:unload, :load]] + all_sharing = Concurrent::CyclicBarrier.new(4) + [load_params, load_params, unload_params, unload_params].permutation do |thread_params| with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch| threads = thread_params.map do |purpose, compatible| Thread.new do @lock.sharing do + all_sharing.wait @lock.exclusive(purpose: purpose, compatible: compatible) do scratch_pad_mutex.synchronize { scratch_pad << purpose } end @@ -209,6 +215,245 @@ class ShareLockTest < ActiveSupport::TestCase end end + def test_new_share_attempts_block_on_waiting_exclusive + with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch| + release_exclusive = Concurrent::CountDownLatch.new + + waiting_exclusive = Thread.new do + @lock.sharing do + @lock.exclusive do + release_exclusive.wait + end + end + end + assert_threads_stuck waiting_exclusive + + late_share_attempt = Thread.new do + @lock.sharing {} + end + assert_threads_stuck late_share_attempt + + sharing_thread_release_latch.count_down + assert_threads_stuck late_share_attempt + + release_exclusive.count_down + assert_threads_not_stuck late_share_attempt + end + end + + def test_share_remains_reentrant_ignoring_a_waiting_exclusive + with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch| + ready = Concurrent::CyclicBarrier.new(2) + attempt_reentrancy = Concurrent::CountDownLatch.new + + sharer = Thread.new do + @lock.sharing do + ready.wait + attempt_reentrancy.wait + @lock.sharing {} + end + end + + exclusive = Thread.new do + @lock.sharing do + ready.wait + @lock.exclusive {} + end + end + + assert_threads_stuck exclusive + + attempt_reentrancy.count_down + + assert_threads_not_stuck sharer + assert_threads_stuck exclusive + end + end + + def test_compatible_exclusives_cooperate_to_both_proceed + ready = Concurrent::CyclicBarrier.new(2) + done = Concurrent::CyclicBarrier.new(2) + + threads = 2.times.map do + Thread.new do + @lock.sharing do + ready.wait + @lock.exclusive(purpose: :x, compatible: [:x], after_compatible: [:x]) {} + done.wait + end + end + end + + assert_threads_not_stuck threads + end + + def test_manual_yield + ready = Concurrent::CyclicBarrier.new(2) + done = Concurrent::CyclicBarrier.new(2) + + threads = [ + Thread.new do + @lock.sharing do + ready.wait + @lock.exclusive(purpose: :x) {} + done.wait + end + end, + + Thread.new do + @lock.sharing do + ready.wait + @lock.yield_shares(compatible: [:x]) do + done.wait + end + end + end, + ] + + assert_threads_not_stuck threads + end + + def test_manual_incompatible_yield + ready = Concurrent::CyclicBarrier.new(2) + done = Concurrent::CyclicBarrier.new(2) + + threads = [ + Thread.new do + @lock.sharing do + ready.wait + @lock.exclusive(purpose: :x) {} + done.wait + end + end, + + Thread.new do + @lock.sharing do + ready.wait + @lock.yield_shares(compatible: [:y]) do + done.wait + end + end + end, + ] + + assert_threads_stuck threads + ensure + threads.each(&:kill) if threads + end + + def test_manual_recursive_yield + ready = Concurrent::CyclicBarrier.new(2) + done = Concurrent::CyclicBarrier.new(2) + do_nesting = Concurrent::CountDownLatch.new + + threads = [ + Thread.new do + @lock.sharing do + ready.wait + @lock.exclusive(purpose: :x) {} + done.wait + end + end, + + Thread.new do + @lock.sharing do + @lock.yield_shares(compatible: [:x]) do + @lock.sharing do + ready.wait + do_nesting.wait + @lock.yield_shares(compatible: [:x, :y]) do + done.wait + end + end + end + end + end + ] + + assert_threads_stuck threads + do_nesting.count_down + + assert_threads_not_stuck threads + end + + def test_manual_recursive_yield_cannot_expand_outer_compatible + ready = Concurrent::CyclicBarrier.new(2) + do_compatible_nesting = Concurrent::CountDownLatch.new + in_compatible_nesting = Concurrent::CountDownLatch.new + + incompatible_thread = Thread.new do + @lock.sharing do + ready.wait + @lock.exclusive(purpose: :x) {} + end + end + + yield_shares_thread = Thread.new do + @lock.sharing do + ready.wait + @lock.yield_shares(compatible: [:y]) do + do_compatible_nesting.wait + @lock.sharing do + @lock.yield_shares(compatible: [:x, :y]) do + in_compatible_nesting.wait + end + end + end + end + end + + assert_threads_stuck incompatible_thread + do_compatible_nesting.count_down + assert_threads_stuck incompatible_thread + in_compatible_nesting.count_down + assert_threads_not_stuck [yield_shares_thread, incompatible_thread] + end + + def test_manual_recursive_yield_restores_previous_compatible + ready = Concurrent::CyclicBarrier.new(2) + do_nesting = Concurrent::CountDownLatch.new + after_nesting = Concurrent::CountDownLatch.new + + incompatible_thread = Thread.new do + ready.wait + @lock.exclusive(purpose: :z) {} + end + + recursive_yield_shares_thread = Thread.new do + @lock.sharing do + ready.wait + @lock.yield_shares(compatible: [:y]) do + do_nesting.wait + @lock.sharing do + @lock.yield_shares(compatible: [:x, :y]) {} + end + after_nesting.wait + end + end + end + + assert_threads_stuck incompatible_thread + do_nesting.count_down + assert_threads_stuck incompatible_thread + + compatible_thread = Thread.new do + @lock.exclusive(purpose: :y) {} + end + assert_threads_not_stuck compatible_thread + + post_nesting_incompatible_thread = Thread.new do + @lock.exclusive(purpose: :x) {} + end + assert_threads_stuck post_nesting_incompatible_thread + + after_nesting.count_down + assert_threads_not_stuck recursive_yield_shares_thread + # post_nesting_incompatible_thread can now proceed + assert_threads_not_stuck post_nesting_incompatible_thread + # assert_threads_not_stuck can now proceed + assert_threads_not_stuck incompatible_thread + end + def test_in_shared_section_incompatible_non_upgrading_threads_cannot_preempt_upgrading_threads scratch_pad = [] scratch_pad_mutex = Mutex.new |