From: "aaronjensen (Aaron Jensen)" <noreply@...>
Date: 2022-01-13T01:12:31+00:00
Subject: [ruby-core:107091] [Ruby master Bug#18474] 938e027c seems to have caused a regression in yield handling with concurrent-ruby

Issue #18474 has been updated by aaronjensen (Aaron Jensen).


Thank you, this is helpful. I'm not sure whether or not concurrent-ruby is the right place to fix this. If someone were doing what your example does in a loop and then joining all of them and making that enumerable, it would be their responsibility to ensure it actually behaves like an enumerable. It may be that the right thing to do is to add `      rescue LocalJumpError` [this line](https://github.com/rails/sprockets/blob/8fc492abde6b20afbe6fa5d5aa7d1085aafd3527/lib/sprockets/manifest.rb#L128). In the case of a local jump all of the promises would still be waited for, which is unfortunate but probably doesn't matter in this specific instance.

The thing that I still do not understand is what it is about concurrent ruby's Promise#wait that allows it to still evaluate the code that is throwing the error. If I understood that better it might point to concurrent-ruby being responsible for respecting LocalJumpErrors.


mame (Yusuke Endoh) wrote in #note-4:
> I think I understand the issue. When the following code is executed on Ruby 3.0, it prints `"should_not_reach_here"`.
> 
> ```
> def foo
>   Thread.new do
>     1.times do
>       yield
>     end
>     p "should_not_reach_here"
>   end.join
> end
> 
> to_enum(:foo).first #=> "should_not_reach_here"
> ```
> 
> This behavior is obviously wrong. Because `Enumerable#first` does `break` internally, it must not reach the line of `p "should_not_reach_here"`. Instead, it should raise LocalJumpError or something like this.
> 
> ```
> def foo
>   Thread.new do
>     1.times do
>       yield
>     end
>     p "should_not_reach_here"
>   end.join
> end
> 
> foo { break } #=> break from proc-closure (LocalJumpError)
> ```
> 
> commit:938e027cdf019ff2cb6ee8a7229e6d9a4d8fc953 fixed this wrong behavior (whether the author @tenderlovemaking intended to or not).
> 
> It is unfortunate that the fix affected concurrent-ruby, but I think this fix should be backported to older ruby branches. I am sorry but I don't know how we can fix concurrent-ruby. I don't know whether it is possible to catch non-local exit event (such as break) in a block that a method yields to.
> 
> ---
> 
> BTW, there is another (more critical) issue. When the code in question is executed on Ruby 3.1, it dumps core.
> 
> ```
> def foo
>   Thread.new do
>     1.times do
>       yield
>     end
>     p "should_not_reach_here"
>   end.join
> end
> 
> to_enum(:foo).first
> #=> #<Thread:0x00007fa51de97108 -:2 run> terminated with exception (report_on_exception is true):
> #   [BUG] Segmentation fault at 0x0000000000000018
> ```
> 
> Ruby 3.0 also dumps core if `1.times do` is removed.
> 
> ```
> def foo
>   Thread.new do
>     yield
>     p "should_not_reach_here"
>   end.join
> end
> 
> to_enum(:foo).first
> #=> #<Thread:0x000055cfd9e60d58 -:2 run> terminated with exception (report_on_exception is true):
> #   [BUG] Segmentation fault at 0x0000000000000018
> ```
> 
> First I thought this is the same issue, but it is different from this ticket. I'll discuss this in #18475.



----------------------------------------
Bug #18474: 938e027c seems to have caused a regression in yield handling with concurrent-ruby
https://bugs.ruby-lang.org/issues/18474#change-95936

* Author: aaronjensen (Aaron Jensen)
* Status: Open
* Priority: Normal
* ruby -v: 3.1.0
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
I'm sorry for the awful title, I don't know enough about what is going on to describe it better. From a debugging perspective it seems that a very specific set of circumstances causes a `begin` block to be jumped out of when a proc it is calling yield, without anything being rescuable. An ensure block will be evaluated, if added, but nothing after that. This is begin block: https://github.com/ruby-concurrency/concurrent-ruby/blob/52c08fca13cc3811673ea2f6fdb244a0e42e0ebe/lib/concurrent-ruby/concurrent/executor/safe_task_executor.rb#L23-L29

For reference, see this issue: https://github.com/ruby-concurrency/concurrent-ruby/issues/931

I have a repro here: https://github.com/aaronjensen/concurrent-repro

It requires concurrent-ruby. Someone better versed than me may be able to create a repro without it, but there is a lot going on in concurrent-ruby.

Everything in the repro seems to be required, to_a.first works, but first does not for example.



-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>