socketry / async

An awesome asynchronous event-driven reactor for Ruby.
MIT License
2.15k stars 89 forks source link

Argument Error on initializing Sync on versions after 2.5.1 when using `request_store-fibers` / `fiber_hook` #358

Open bewatts opened 20 hours ago

bewatts commented 20 hours ago

I'm in the middle of upgrading a rails app from rails 6.1.7.9 to 7.2, and getting some very odd behavior when I'm trying to upgrade the async gem from 2.5.1 to 2.20.0. I'm seeing similar behavior on versions ~2.6 as well. My use of the Sync method seems consistent with the documentation.

On version 2.5.1:

irb(main):001> require 'async'
=> false
irb(main):002>
irb(main):003* def wat
irb(main):004*   begin
irb(main):005*     Sync { pp '?? wat?? '}
irb(main):006*   rescue => e
irb(main):007*     pp e.backtrace
irb(main):008*     throw e
irb(main):009*   end
irb(main):010> end
=> :wat
irb(main):011>
irb(main):012> wat
irb(main):013>
"?? wat?? "
=> "?? wat?? "
irb(main):014>

On version 2.20.0:

rails_console* def wat
rails_console*   begin
rails_console*     Sync {'?? wat?? '}
rails_console*   rescue => e
rails_console*     pp e.backtrace
rails_console*   end
rails_console end
rails_console
rails_console wat
rails_console

StackTrace

["./.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/fiber-annotation-0.2.0/lib/fiber/annotation.rb:13:in `initialize'",
 "./.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/fiber_hook-0.1.0/lib/fiber_hook.rb:59:in `new'",
 "./.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/fiber_hook-0.1.0/lib/fiber_hook.rb:59:in `new'",
 "./.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/async-2.20.0/lib/async/task.rb:433:in `schedule'",
 "./.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/async-2.20.0/lib/async/task.rb:196:in `run'",
 "./.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/async-2.20.0/lib/async/scheduler.rb:497:in `async'",
 "./.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/async-2.20.0/lib/async/scheduler.rb:473:in `run'",
 "./.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/async-2.20.0/lib/kernel/sync.rb:33:in `Sync'",
 "(rails_console):19:in `wat'",
 "(rails_console):26:in `<main>'",
ioquatix commented 20 hours ago

Thanks for your report, I'll take a look.

ioquatix commented 20 hours ago

Do you mind explaining this line of code:

irb(main):008*     throw e

in other words, what are your intention of using throw without catch?

bewatts commented 20 hours ago

Oh, just explanatory, non interesting to the example. This was my attempt to show the simplest example that re-created what I'm seeing my production code, which is erroring on calling the Sync method (without the annotation keyword arg).

ioquatix commented 20 hours ago

Okay, I see. I think maybe you meant to use raise instead of throw - even if it's just the example code... it threw me off :)

ioquatix commented 20 hours ago

Okay, I think the issue is with fiber_hook and/or fiber-annotate. Let me take a look.

ioquatix commented 19 hours ago

Reproduction:

require 'bundler/inline'

gemfile do
    source 'https://rubygems.org'
    gem 'async'
    gem 'fiber_hook'
end

require 'fiber_hook'

Sync{'?? wat?? '}
ioquatix commented 19 hours ago

The fiber-hook gem might be fixable by passing through arguments, however, I advise you to avoid it, and probably also request_store-fibers too if possible. Instead, use Fiber#storage feature in Ruby.

My general advice would be:

  1. RequestStore by itself is okay, but it won't inherit to child Async tasks (or threads).
  2. If you need that behaviour, replace RequestStore.store[x] with Fiber[x].
require 'bundler/inline'

gemfile do
    source 'https://rubygems.org'
    gem 'async'
    gem 'request_store'
    gem 'stringio'
end

RequestStore.store[:foo] = 'bar'
puts "RequestStore.store[:foo] = #{RequestStore.store[:foo].inspect} (outer)"

Fiber[:foo] = 'bar'
puts "Fiber[:foo] = #{Fiber[:foo].inspect} (outer)"

puts "Async do ..."
Async do
    puts "RequestStore.store[:foo] = #{RequestStore.store[:foo].inspect} (inner)"
    puts "Fiber[:foo] = #{Fiber[:foo].inspect} (inner)"
end