Unexpected thread behavior inside debugger context

[mhoran]

In capybara-webkit, we use threads to work around a JRuby bug in IO.select. This causes an issue when commands are sent to the webkit_server process from within a debugger context. The issue persists on master, so I'm opening this issue to document my findings.

A simple test case does not behave as expected:

require 'byebug'

def read
  Thread.abort_on_exception = true
  Thread.new {}.join
end

def main
  byebug
end

main

Calling read from main results in test.rb:5:in join': No live threads left. Deadlock? (fatal)`. While seemingly related to #54, this issue persists on master.

Here's a test case that's more similar to what we do in capybara-webkit:

require 'socket'
fork do
  server = TCPServer.new 3001
  client = server.accept
  client.gets
end

require 'byebug'

def connect
  Timeout.timeout(5) do
    while @socket.nil?
      attempt_connect
    end
  end
end

def attempt_connect
  @socket = TCPSocket.open("127.0.0.1", 3001)
rescue Errno::ECONNREFUSED
end

def read
  response = ''
  begin
    while response.length < 1 do
      response += @socket.read_nonblock(1)
    end
  rescue IO::WaitReadable
    Thread.abort_on_exception = true
    Thread.new { IO.select([@socket]) }.join
    retry
  end
end

def main
  connect
  byebug
end

main

This results, again, in test.rb:31:in join': No live threads left. Deadlock? (fatal)`.

Oddly, when capybara-webkit is utilized by a project and the #read method is invoked, one does not see this error but instead, the debugger hangs. This has been documented in deivid-rodriguez/pry-byebug#51. Unfortunately, I've been unable to isolate this behavior, but it is reproducible with the latest version of capybara-webkit.

[deivid-rodriguez]

@mhoran Thanks, that's very helpful. I'll get to this soon!

[deivid-rodriguez]

I'll commit a fix for this in the next days.

[deivid-rodriguez]

So I've fixed the issue you reported but I'm pretty sure the other issue will still happen. So what would be the steps to reproduce it?

The issue you linked to seems to the same as the one I've fixed (because they both happen when evaluating stuff from byebug/pry-byebug's prompt) although it is true that the errors are different.

[mhoran]

I've been unable to extract a simple test case, but I do have a simple app that exhibits the behavior.

Clone the app and check out the byebug-bug branch. Running spec/features/checkbox_spec.rb should drop you into a debugger, at which point p page.body will hang indefinitely. Interestingly, page.body and puts page.body.inspect both work fine.

There's nothing special about the app; it's just a Rails app that I test capybara-webkit features against. I see this behavior in Ruby 2.0 through 2.2. This behavior should also be reproducible with a fresh Rails app, but I've set this one up with examples.

If you have any questions about the internals of capybara-webkit and what may be causing the hang, let me know. However, the sample code above basically represents the related capybara-webkit implementation. Removing Thread.new { IO.select([@socket]) }.join from lib/capybara/webkit/connection.rb does resolve the issue. This leads to other problems on JRuby, unfortunately.

[deivid-rodriguez]

I've added some other commits that should fix the other issue. It was the same issue but my first fix was not complete.

I think all issues reported here happen when evaluating stuff from byebug's prompt, and not while the debugger is normally running users code. In that case, I think we are done here! :) Let me know if it works!

[mhoran]

Awesome! This looks to be working now. I didn't realize that p was a byebug command, so that explains a few things.

One thing that still hangs is visit '/' from the byebug prompt in the sample app. This is a weird one, and I wouldn't necessarily expect it to work. When running a test in Capybara, a WEBrick server is spun up in another thread. capybara-webkit will then make a request to that server from an external process, and the Capybara DSL can be used to inspect the page.

The WEBrick thread will normally be running by the time we get to the debug prompt, so long as a visit has occurred before byebug. If I move byebug above visit '/checkbox.html' in the sample app, I don't get a hang.

[deivid-rodriguez]

@mhoran I've added a failing test case for the new issue. It looks like a valid one to me. I think whatever pry is able to evaluate, byebug should be able to do it as well.

I'm not sure how to fix it though, it looks like my first fix is not valid and I need to rethink the whole thing. Hope to come up with something in the next days.

[deivid-rodriguez]

@mhoran It should be fixed now! Please don't find yet another bug that invalids everything 😄

Actually I haven't tried your sample app, only my test case. Could you try the sample app and confirm it's working?

[mhoran]

Sample app confirms all is well. Thanks for working through this!

[deivid-rodriguez]

Thanks for identifying the issues and making them easy to reproduce!

[andrewfader]

@deivid-rodriguez I spoke to @mhoran about this and I assume it's the same issue we have with byebug 5.0, pry-byebug 3.1 and the heads of master for those. See also deivid-rodriguez/pry-byebug#51. Basically we get a hang when doing a "binding.pry" on a capybara-webkit spec run. Webkit debug indicates that it returned, e.g., "page.driver.version," but we never see the output on the debugger.

[deivid-rodriguez]

I'm going to reopen this cause there's something definitely not quite working here. I've also run very sporadicly into deadlocks with the latests versions.

[deivid-rodriguez]

@andrewfader I've pushed some drastic changes in how byebug is implemented internally. I've run the test suite in a loop to try to reproduce the deadlocks, but they don't seem to be happening anymore (before it used to deadlock once every 20 runs or so). I'm not very positive because there's no changes really in how threads are handled, but you might want to give latest master a try.

[andrewfader]

@deivid-rodriguez Thanks! Unfortunately these changes seem to break pry-byebug. I made a pull request to fix an error I was getting on Gemfile require about Byebug::Processor, but it now complains that "undefined method `handler=' for Byebug:Module" when I try to do a binding.pry. I assume there are some more changes that need to be done in pry-byebug

[deivid-rodriguez]

Of course, I meant you to try just byebug. But no problem, I'll push a pry-byebug branch supporting lastest byebug master. Hold on.

[andrewfader]

Thanks! Appreciate the prompt replies and fixes!

[deivid-rodriguez]

Try https://github.com/deivid-rodriguez/pry-byebug/tree/support_latest_byebug. You have to explicitly add gem 'byebug', github: 'deivid-rodriguez/byebug' to your Gemfile.

[andrewfader]

Gem works now but I'm not sure that the hanging has been fixed. I get into a pry like this:

[1] pry(#RSpec::ExampleGroups::...)> page.driver.version
Received "Version()"
Started "Version()"
Finished "Version()" with response "Success(Qt: 5.4.2
WebKit: 538.1
QtWebKit: 5.4.2)"
Wrote response true "Qt: 5.4.2
WebKit: 538.1
QtWebKit: 5.4.2"

Then it hangs. Could be a problem lower down? Capybara-puma, capybara-webkit etc? Using:

gem 'pry-byebug', github: 'deivid-rodriguez/pry-byebug', branch: 'support_latest_byebug'
gem 'pry', github: 'pry/pry'
gem 'byebug', github: 'deivid-rodriguez/byebug'

[deivid-rodriguez]

So basically you're getting the same behavior as before, right?

[andrewfader]

Seems that way

[deivid-rodriguez]

Ok. I won't get to this in a while, but if you can create a repo with instructions to reproduce, it'd be great.

[andrewfader]

So I made a small repo here: https://github.com/andrewfader/minimal_byebug_hang

bash-3.2$ rspec
hello
Capybara: 2.4.4
capybara-webkit: 1.6.0
Qt: 5.4.2
WebKit: 538.1
QtWebKit: 5.4.2

From: /Users/pivotal/workspace/minimal_byebug_hang_repo/spec/capybara_test_spec.rb @ line 10 :

     5:   describe 'helo' do
     6:     it 'hi' do
     7:       puts "hello"
     8:       puts page.driver.version
     9:       binding.pry
 => 10:       puts 'we aint here'
    11:     end
    12:   end
    13: end

[1] pry(#<RSpec::ExampleGroups::ATest::Helo>)> page.driver.version

[deivid-rodriguez]

Hei, great. Last thing, is this reproducible using byebug only? It seems like it is. In that case, it'd be good to update the repo and remove pry and pry-byebug dependencies.

[andrewfader]

Interesting, no, it actually seems to work fine if I remove pry and pry-byebug and use a debugger statement instead.

[4, 13] in /Users/pivotal/workspace/minimal_byebug_hang_repo/spec/capybara_test_spec.rb
    4: feature "A test", js: true do
    5:   describe 'helo' do
    6:     it 'hi' do
    7:       puts "hello"
    8:       puts page.driver.version
    9:       debugger
=> 10:       puts 'we aint here'
   11:     end
   12:   end
   13: end
(byebug) page.driver.version
Capybara: 2.4.4
capybara-webkit: 1.6.0
Qt: 5.4.2
WebKit: 538.1
QtWebKit: 5.4.2
(byebug)

[deivid-rodriguez]

Interesting indeed, but good news too.

Could you reopen this in pry-byebug linking to your sample repo?

[CamJN]

This still happens to me and I don't use pry in any way

byebug (5.0.0)
capybara (2.4.4)
capybara-webkit (1.6.0)
qt5: stable 5.5.0
WebKit: 538.1
QtWebKit: 5.5.0

...
debugger
find '#article_image' # <- hangs when called from byebug

[deivid-rodriguez]

Reproduce against latest master if you want this reopened, please.

[CamJN]

It is fixed on master, is there going to be a release soon?

[deivid-rodriguez]

Yes!

[mcfiredrill]

Sorry if I missed it, but what commit(s) was this fixed in?

[deivid-rodriguez]

I don't remember. If you have trouble with the latest version, just open a separate issue.