Skip to content

Commit

Permalink
Fixes #54, thanks @zmoazeni!
Browse files Browse the repository at this point in the history
Always return an integer when asking for stack sizes because "when we
convert the nil to an int in C bad things happen"...
  • Loading branch information
David Rodríguez de Dios committed Apr 12, 2014
1 parent 35b2ca6 commit 39f8763
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 13 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* Fix post_mortem mode
* Command history is also saved after regular program termination, not only
after quitting with the `q` command.
* Fix failure when calling Byebug.start with Timeout::timeout (see #54), thanks
@zmoazeni!

- Changes
* History file is `per project` by default now
Expand Down
2 changes: 1 addition & 1 deletion ext/byebug/context.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ context_mark(void *data)
static int
real_stack_size()
{
return FIX2INT(rb_funcall(cContext, rb_intern("real_stack_size"), 0));
return FIX2INT(rb_funcall(cContext, rb_intern("stack_size"), 1, Qtrue));
}

extern VALUE
Expand Down
20 changes: 8 additions & 12 deletions lib/byebug/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,20 @@ module Byebug
class Context

class << self
def stack_size
def stack_size(byebug_frames = false)
if backtrace = Thread.current.backtrace_locations(0)
backtrace.drop_while { |l| !ignored(l.path) }
.drop_while { |l| ignored(l.path) }
.take_while { |l| !ignored(l.path) }
.size
unless byebug_frames
backtrace = backtrace.drop_while { |l| !ignored(l.path) }
.drop_while { |l| ignored(l.path) }
.take_while { |l| !ignored(l.path) }
end
backtrace.size
else
print 'No backtrace available!!'
print 'WARNING: No backtrace available!!'
0
end
end

def real_stack_size
if backtrace = Thread.current.backtrace_locations(0)
backtrace.size
end
end

def ignored(path)
IGNORED_FILES.include?(path)
end
Expand Down

11 comments on commit 39f8763

@zmoazeni
Copy link

Choose a reason for hiding this comment

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

@deivid-rodriguez I think something in this commit causes a tailspin in the app that I was having trouble with #54 I'm trying to reproduce

@zmoazeni
Copy link

Choose a reason for hiding this comment

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

I'm trying to reproduce in master but this hangs locally when I use master (as of d8a2e36):

∴ be irb
2.1.1 :001 > require 'byebug'
 => true 
2.1.1 :002 > Byebug.start
# this never returns and I have to force quit the process

@deivid-rodriguez
Copy link
Owner

Choose a reason for hiding this comment

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

:( just my luck!

And does it not happen if you apply you simpler patch? Thanks a lot for looking into this, I'm frustrated because I've never been able to reproduce it...

@zmoazeni
Copy link

Choose a reason for hiding this comment

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

Yeah, this is a weird one because I can't reproduce the same thing on the byebug master repo directly or when I point my app to the local repo with the gem 'byebug', path: '...' option.

@deivid-rodriguez
Copy link
Owner

Choose a reason for hiding this comment

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

So how exactly are you reproducing it? Only in your local fork?

@zmoazeni
Copy link

Choose a reason for hiding this comment

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

Inside the app that had trouble with #54, I added this to the Gemfile:

gem 'byebug', github: 'deivid-rodriguez/byebug' and ran the stuff above in 39f8763#commitcomment-5992753

I also pointed it to this SHA to see if it might have been d8a2e36. It fails on both.

gem 'byebug', github: 'deivid-rodriguez/byebug', ref: '39f8763c676ecaeb836a8c7a6c19b66f87b6b221'

@deivid-rodriguez
Copy link
Owner

Choose a reason for hiding this comment

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

So gem ' byebug' and gem 'byebug', path: '...' work, but gem 'byebug', github: 'deivid-rodriguez/byebug' and
gem 'byebug', github: 'deivid-rodriguez/byebug', ref: '39f8763c676ecaeb836a8c7a6c19b66f87b6b221' don't?

This is so weird...

@zmoazeni
Copy link

Choose a reason for hiding this comment

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

Yeah, exactly. I'm trying to figure out if bundler compiles byebug differently when it clones/builds it. Here's what I'm seeing in a new folder (not tied to the app I'm working on) and no versions of byebug installed locally:

# A Gemfile

## To reproduce
# be irb
#2.1.1 :001 > require 'byebug'
#  => true
#2.1.1 :002 > Byebug.start

# This doesn't work. Tails spins after the Byebug.start
gem 'byebug', github: 'deivid-rodriguez/byebug', ref: '39f8763c676ecaeb836a8c7a6c19b66f87b6b221'

# If I run `rake clean compile` this works fine
gem 'byebug', path: '/Users/zmoazeni/projects/byebug'

# Off a fresh clone of master, `rake install` this works fine
gem 'byebug'

@zmoazeni
Copy link

Choose a reason for hiding this comment

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

Wow, I think it was a bug in bundler version 1.5.3. I just upgraded to the latest version of bundler 1.6.1 and all three variations above work fine now.

Sorry about the spam.

@zmoazeni
Copy link

Choose a reason for hiding this comment

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

Yeah I think it's related to rubygems/bundler#2847 and rubygems/bundler@9007c94

I see that behavior with rubygems v2.2.2 and bundler v1.6.0, but it's fine with bundler v1.6.1. I guess this might be a heads up in case anyone else runs into the same thing and think it's a byebug issue.

@deivid-rodriguez
Copy link
Owner

Choose a reason for hiding this comment

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

Great, thanks again! And no worries for the spam, as you said, it might turn out to be useful for someone!

Please sign in to comment.