ruby / debug

Debugging functionality for Ruby
BSD 2-Clause "Simplified" License
1.12k stars 122 forks source link

Debugging Async tasks yields `RuntimeError: running is given, but running` #486

Open machty opened 2 years ago

machty commented 2 years ago

Your environment

Describe the bug

Debugging programs with the Async library often leads to RuntimeError: running is given, but running

To Reproduce

Install async 2.0.0:

gem install async

Create ruby script async_debug_test_case.rb as follows:

require 'async'

Async do |t|
  t.async do
    puts "1\n"
  end
  t.async do
    puts "2\n"
  end
end

Run rdbg, set breakpoints on both putss, and continue until error is raised. See output below:

rdbg scratch/async_debug_test_case.rb 
[1, 10] in scratch/async_debug_test_case.rb
=>   1| require 'async'
     2| 
     3| Async do |t|
     4|   t.async do
     5|     puts "1\n"
     6|   end
     7|   t.async do
     8|     puts "2\n"
     9|   end
=>#0    <main> at scratch/async_debug_test_case.rb:1
(rdbg) b 5    # break command
#0  BP - Line  /Users/machty/code/exc/rails/scratch/async_debug_test_case.rb:5 (line)
(rdbg) b 8    # break command
#1  BP - Line  /Users/machty/code/exc/rails/scratch/async_debug_test_case.rb:8 (line)
(rdbg) c    # continue command
[1, 10] in scratch/async_debug_test_case.rb
     1| require 'async'
     2| 
     3| Async do |t|
     4|   t.async do
=>   5|     puts "1\n"
     6|   end
     7|   t.async do
     8|     puts "2\n"
    10| end
=>#0    block in <main> (2 levels) at scratch/async_debug_test_case.rb:5
  #1    block in schedule at ~/.gem/ruby/3.1.0/gems/async-2.0.0/lib/async/task.rb:258

Stop by #0  BP - Line  /Users/machty/code/exc/rails/scratch/async_debug_test_case.rb:5 (line)
(rdbg) c    # continue command
[3, 11] in scratch/async_debug_test_case.rb
     3| Async do |t|
     4|   t.async do
     5|     puts "1\n"
     6|   end
     7|   t.async do
=>   8|     puts "2\n"
     9|   end
    10| end
    11| 
=>#0    block in <main> (2 levels) at scratch/async_debug_test_case.rb:8
  #1    block in schedule at ~/.gem/ruby/3.1.0/gems/async-2.0.0/lib/async/task.rb:258

Stop by #1  BP - Line  /Users/machty/code/exc/rails/scratch/async_debug_test_case.rb:8 (line)
1
(rdbg) c    # continue command
["DEBUGGER Exception: /Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:986",
 #<RuntimeError: running is given, but running>,
 ["/Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:115:in `set_mode'",
  "/Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:698:in `wait_next_action_'",
  "/Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:676:in `wait_next_action'",
  "/Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:284:in `suspend'",
  "/Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:217:in `on_breakpoint'",
  "/Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/breakpoint.rb:65:in `suspend'",
  "/Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/breakpoint.rb:157:in `block in setup'",
  "scratch/async_debug_test_case.rb:8:in `block (2 levels) in <main>'",
  "/Users/machty/.gem/ruby/3.1.0/gems/async-2.0.0/lib/async/task.rb:258:in `block in schedule'"]]
  0.0s    error: Async::Task [oid=0x5c8] [ec=0x5dc] [pid=34553] [2022-01-12 04:52:04 -0400]
               |   RuntimeError: running is given, but running
               |   → /Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:115 in `set_mode'
               |     /Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:698 in `wait_next_action_'
               |     /Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:676 in `wait_next_action'
               |     /Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:284 in `suspend'
               |     /Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/thread_client.rb:217 in `on_breakpoint'
               |     /Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/breakpoint.rb:65 in `suspend'
               |     /Users/machty/.rubies/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/debug-1.4.0/lib/debug/breakpoint.rb:157 in `block in setup'
               |     scratch/async_debug_test_case.rb:8 in `block (2 levels) in <main>'
               |     /Users/machty/.gem/ruby/3.1.0/gems/async-2.0.0/lib/async/task.rb:258 in `block in schedule'

Expected behavior

Debugging should not crash.

Additional context

I can't replicate this issue using pure Fibers, so I suspect this has something to do with Ruby's new Fiber Scheduler interface and the way the Async library uses it. Note that simpler test cases using a single test cases involving one task don't seem to cause the rror.

st0012 commented 2 years ago

I think the debugger generally is not fiber-safe due to the current design. I also found https://github.com/ruby/debug/issues/490 when debugging this issue. I guess this issue is caused by a similar reason: a fiber is resumed in the middle of debugger operation, so its state is changed unexpectedly.

ko1 commented 2 years ago

Now Fiber is not supported well. Need more investigation.

kingdonb commented 2 years ago

It is the same for the FiberScheduler, as I can see it:

VSCode debugger breakpoints that get set within a fiber are not only never reached, the fibers don't even run

It will be pretty awesome if we can use fibers and debug together, but with this limitation in mind I can work around it for my debugging.

Anyway I am not sure if

st0012 commented 2 years ago

@kingdonb #490 was one of the Fiber issues at the time but not the whole of it. And it's indeed addressed so I closed it. But perhaps the name was too general so it's confusing 😅

trevorturk commented 2 years ago

I ran into this issue while upgrading an app from Ruby 2.7.6 to 3.1.2, also Async 1.30.3 to 2.0.3, even when not invoking the debugger. I'm able to work around the issue by removing debug from my Gemfile, but of course that's not ideal.

I assume there will be some rough edges as Fibers continue to gain support, so no worries. It sounds like it may be a heavy lift for the debugger (re: the comment above "debugger generally is not fiber-safe due to the current design") but I'm curious if anyone knows of a workaround, perhaps a different debugger gem for now? I'm also happy to provide additional details if there's interest in working on this. I'll also ping the Socketry org that works on Async as well to see if anyone has ideas.

Thank you!

trevorturk commented 2 years ago

Just a note from @st0012's suggestion to test byebug and although it says "Ruby 2" on the GitHub repo, it seems to work fine in my testing.

olivierlacan commented 1 year ago

Under Ruby 3.2.2 while running several Async::Task (2.5.0) which include a binding.irb I don't end up with multiple prompts as shown in the OP but with one prompt interacting with all parallel tasks all at once.

This leads to keyboard input being sent to different fibers on each keystroke, for example typing help given two tasks:

So the only way to interact with the tasks using the IRB prompt is to double type each character.

Being able to detect that multiple binding.irb sessions are open within two tasks/fibers and at the very least offering a warning to users would be extremely helpful I think.

Going even further, it seems like an ideal flow would be allow users to select which task/fiber they're interacting with (dRB-style) when debugging, but of course I have no idea if this is realistic or feasible.

ioquatix commented 1 year ago

If you want to run a binding.irb session that blocks the event loop completely, you can do this:

Fiber.blocking{binding.irb}

This will prevent multiple sessions or any async code from running. It will also prevent you from running async code from within the IRB session.

If you want more fine grained control, you could use a mutex

IRB_MUTEX = Thread::Mutex.new

IRB_MUTEX.synchornize{binding.irb}

This will only allow one binding.irb session at a time but won't prevent other async code from executing.

ioquatix commented 1 year ago
  class ThreadClient
    def self.current
      if thc = Thread.current[:DEBUGGER__ThreadClient]
        thc
      else
        thc = SESSION.get_thread_client
        Thread.current[:DEBUGGER__ThreadClient] = thc
      end
    end

seems wrong. Thread.current[:x] is fiber local. I'll try to fix it.

ioquatix commented 1 year ago

I don't know if this solves all the issues but this is a first pass at the issue: https://github.com/ruby/debug/pull/987 and the example given by @machty now works on my machine locally (i.e. it doesn't crash with set_mode :running).

olivierlacan commented 1 year ago

@ioquatix The Fiber.blocking{binding.irb} tip is incredibly helpful but it makes me wonder why this shouldn't be the default behavior in any Async execution context? It's hard to imagine any IRB debugging session working well if it's executed in parallel across fibers in a majority of cases, right?

Wouldn't it make sense to hook into binding.irb to always make it blocking when debugging, if it's possible? And perhaps display a warning that the behavior can be disabled for truly async debugging.

As a side note Fiber.blocking{binding.irb} didn't appear to prevent other async code from running, it executed sequentially after I exited each debugging session.

ioquatix commented 1 year ago

When you enter Fiber.blocking{...}, for a given thread with a fiber scheduler, that thread will then no longer route blocking operations to the event loop. In effect, during the Fiber.blocking{... here ...}, the event loop will not execute. Once you exit Fiber.blocking{...} the event loop will continue as normal. I assume that's what you mean when you said "it executed sequentially after I exited each debugging session." - you are 100% correct.

Regarding binding.irb, the same problem will exist if you do the following:

3.times do
 Thread.new{binding.irb}
end

# Similar issues to:

```ruby
Async do |task|
  3.times do
    task.async{binding.irb}
  end
end

I would say that binding.irb should have a single instance somewhere which is bound to stdin and stdout, and binding.irb should use that shared state to prevent multiple threads from entering e.g. using a Thread::Mutex makes the most sense to me, in effect:

class IRB::Session
  def initialize(input = $stdin, output = $stdout) # ...
    @guard = Thread::Mutex.new
    # ...
  end

  def run
    # Only one thread/fiber can enter here:
    @guard.synchronize{... run the actual UI ...}
  end

  GLOBAL = new
end

def binding.irb
  IRB::Session::GLOBAL.run
end

Or something to that effect...

louim commented 11 months ago

Hello, I just hit the same issue, coming around from https://github.com/socketry/async/issues/225. I use ruby 3.1. Reading the thread, I have the same problem as @trevorturk. I'm not trying to run debug and I don't have any breakpoints set, yet I get a debugger error.

My current work around is gem "debug", require: false and

require "debug"
debugger

when I want to debug. I'd be ok with knowing I cannot debug async code, but being able to load my code without triggering this error.