Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Hash#each family usable in Ractor #4768

Merged
merged 1 commit into from Aug 26, 2021
Merged

Conversation

kou
Copy link
Member

@kou kou commented Aug 24, 2021

We don't need to increment/decrement iteration level for frozen Hash
because frozen Hash can't be modified. We can assume that nobody
changes the target Hash while calling #each family.

How to reproduce:

a = {}
100.times do |i|
  a[i] = true
end
Ractor.make_shareable(a)

4.times.collect do
  Ractor.new(a) do |b|
    100.times do
      b.each_value do
      end
    end
  end
end.each(&:take)

Example output:

internal:ractor>:267: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues.
#<Thread:0x00007fcfb087bb30 run> terminated with exception (report_on_exception is true):
#<Thread:0x00007fcfb087b8d8 run> terminated with exception (report_on_exception is true):
#<Thread:0x00007fcfb088d678 run> terminated with exception (report_on_exception is true):
#<Thread:0x00007fcfb087bd88 run> terminated with exception (report_on_exception is true):
/tmp/h.rb:10:in `each_value'/tmp/h.rb:10:in `each_value': : /tmp/h.rb:10:in `each_value'no implicit conversion from nil to integer/tmp/h.rb:10:in `each_value'no implicit conversion from nil to integer (: :  (TypeErrorTypeError)no implicit conversion from nil to integer)no implicit conversion from nil to integer (

 (TypeErrorTypeError	from /tmp/h.rb:10:in `block (3 levels) in <main>'
	from /tmp/h.rb:10:in `block (3 levels) in <main>'
))	from /tmp/h.rb:9:in `times'

	from /tmp/h.rb:9:in `times'

	from /tmp/h.rb:9:in `block (2 levels) in <main>'
	from /tmp/h.rb:10:in `block (3 levels) in <main>'
	from /tmp/h.rb:10:in `block (3 levels) in <main>'
	from /tmp/h.rb:9:in `block (2 levels) in <main>'
	from /tmp/h.rb:9:in `times'
	from /tmp/h.rb:9:in `times'
	from /tmp/h.rb:9:in `block (2 levels) in <main>'
	from /tmp/h.rb:9:in `block (2 levels) in <main>'
<internal:ractor>:694:in `take': thrown by remote Ractor. (Ractor::RemoteError)
	from /tmp/h.rb:14:in `each'
	from /tmp/h.rb:14:in `<main>'
/tmp/h.rb:10:in `each_value': no implicit conversion from nil to integer (TypeError)
	from /tmp/h.rb:10:in `block (3 levels) in <main>'
	from /tmp/h.rb:9:in `times'
	from /tmp/h.rb:9:in `block (2 levels) in <main>'

@kou
Copy link
Member Author

kou commented Aug 24, 2021

@ko1 @nobu What do you think about this change?

hash.c Outdated
@@ -1503,11 +1503,17 @@ rb_hash_foreach(VALUE hash, rb_foreach_func *func, VALUE farg)

if (RHASH_TABLE_EMPTY_P(hash))
return;
hash_iter_lev_inc(hash);
if (!RB_OBJ_FROZEN(hash))
hash_iter_lev_inc(hash);
Copy link
Member

@nobu nobu Aug 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could move into the else block below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right.
I'll do.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

We don't need to increment/decrement iteration level for frozen Hash
because frozen Hash can't be modified. We can assume that nobody
changes the target Hash while calling #each family.

How to reproduce:

    a = {}
    100.times do |i|
      a[i] = true
    end
    Ractor.make_shareable(a)

    4.times.collect do
      Ractor.new(a) do |b|
        100.times do
          b.each_value do
          end
        end
      end
    end.each(&:take)

Example output:

    internal:ractor>:267: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues.
    #<Thread:0x00007fcfb087bb30 run> terminated with exception (report_on_exception is true):
    #<Thread:0x00007fcfb087b8d8 run> terminated with exception (report_on_exception is true):
    #<Thread:0x00007fcfb088d678 run> terminated with exception (report_on_exception is true):
    #<Thread:0x00007fcfb087bd88 run> terminated with exception (report_on_exception is true):
    /tmp/h.rb:10:in `each_value'/tmp/h.rb:10:in `each_value': : /tmp/h.rb:10:in `each_value'no implicit conversion from nil to integer/tmp/h.rb:10:in `each_value'no implicit conversion from nil to integer (: :  (TypeErrorTypeError)no implicit conversion from nil to integer)no implicit conversion from nil to integer (

     (TypeErrorTypeError	from /tmp/h.rb:10:in `block (3 levels) in <main>'
    	from /tmp/h.rb:10:in `block (3 levels) in <main>'
    ))	from /tmp/h.rb:9:in `times'

    	from /tmp/h.rb:9:in `times'

    	from /tmp/h.rb:9:in `block (2 levels) in <main>'
    	from /tmp/h.rb:10:in `block (3 levels) in <main>'
    	from /tmp/h.rb:10:in `block (3 levels) in <main>'
    	from /tmp/h.rb:9:in `block (2 levels) in <main>'
    	from /tmp/h.rb:9:in `times'
    	from /tmp/h.rb:9:in `times'
    	from /tmp/h.rb:9:in `block (2 levels) in <main>'
    	from /tmp/h.rb:9:in `block (2 levels) in <main>'
    <internal:ractor>:694:in `take': thrown by remote Ractor. (Ractor::RemoteError)
    	from /tmp/h.rb:14:in `each'
    	from /tmp/h.rb:14:in `<main>'
    /tmp/h.rb:10:in `each_value': no implicit conversion from nil to integer (TypeError)
    	from /tmp/h.rb:10:in `block (3 levels) in <main>'
    	from /tmp/h.rb:9:in `times'
    	from /tmp/h.rb:9:in `block (2 levels) in <main>'
@kou
Copy link
Member Author

kou commented Aug 26, 2021

CI is green.
I'll merge this.

@kou kou merged commit a2ad0cb into ruby:master Aug 26, 2021
@kou kou deleted the hash-each-ractor-safe branch August 26, 2021 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants