ahoward / systemu

univeral capture of stdout and stderr and handling of child process pid for windows, *nix, etc.
Other
126 stars 33 forks source link

Possible forkbomb #23

Open chetan opened 12 years ago

chetan commented 12 years ago

Hey,

I ran into a situation where I'm trying to require systemu directly on the command line like so:

RUBYOPT="-rsystemu" ./foo.rb

The following code from systemu.rb causes an infinite loop:

  c = begin; ::RbConfig::CONFIG; rescue NameError; ::Config::CONFIG; end
  ruby = File.join(c['bindir'], c['ruby_install_name']) << c['EXEEXT']
  @ruby = if system('%s -e 42' % ruby)
    ruby
  else
    system('%s -e 42' % 'ruby') ? 'ruby' : warn('no ruby in PATH/CONFIG')
  end

I'm not really sure what this is trying to do; make sure the initially detected ruby binary actually exists? Make sure it's actually ruby? Since the output isn't being verified, I'm not sure this is effective. I think a simple File.exists? test should be good enough, or else maybe this could be lazy loaded the first time the @ruby var is accessed?

I've created a simple test repo here to demonstrate the issue:

https://github.com/chetan/systemu_forkbomb

Simply run test.sh or test.rb and watch ps to see what happens. The only way to kill it is killall ruby.

chetan

chetan commented 12 years ago

Here's the fix I'm going with for now:

https://github.com/chetan/systemu/commit/cb6524a4c50213bec52262f6c0f090e11bb3a586

Let me know if that looks ok. Another possibility would be to keep the old code but modify ENV["RUBYOPT"] before and after, but I prefer the method above.

damphyr commented 12 years ago

I don't think your solution will work on Windows. Well, at least the 'which' part won't for sure, but I'll try and find sometime to test it today or tomorrow

chetan commented 12 years ago

Good point, I didn't really consider windows. In any case, what are the situations where Config/RbConfig do not contain the correct runtime? Perhaps it's best to simply set it to "ruby" in that case and throw an error when needed instead of warning immediately.

ahoward commented 11 years ago

@chetan the whole point of systemu is to be portable, including on windows. RbConfig is often blown on windows or any platform where people sometimes do not compile and install a binary distribution: RbConfig is generated during the build process so it always reflects the builder's machine - not the installed on machine...

therefore pushing and poping RUBYOPT is the best bet.