Bundler exec with Ruby 2.0 FD defaults

[steved]

Just wondering if there was any thought towards bundle exec on 2.0 being backwards compatible with 1.9 in terms of file descriptor inheritance. Essentially making this change in bundler/cli.rb:

args << { :close_others => false } if RUBY_VERSION > '1.9'
Kernel.exec(*args)

[indirect]

Huh. I didn't realize that there was a change in descriptor inheritance. Do you have a use-case where it matters either way?

On Sep 3, 2013, at 5:00 PM, Steven Davidovitz notifications@github.com wrote:

Just wondering if there was any thought towards bundle exec on 2.0 being backwards compatible with 1.9 in terms of file descriptor inheritance. Essentially making this change in bundler/cli.rb:

args << { :close_others => false } if RUBY_VERSION > '1.9' Kernel.exec(*args) — Reply to this email directly or view it on GitHub.

[gnufied]

+1 here. It is probably a good idea to adopt Ruby 2.0 behaviour anyways (seems saner!) .

[steved]

I may have mis-stated it slightly. We'd need to be backwards compatible with 1.9 even on 2.0. I personally ran into this today. It boiled down to soft-restarting unicorn while trying to re-execute it with bundler.

If you have any familiarity with unicorn, if you set the START_CTX variable to something akin to:

bundle exec unicorn_rails ...

it will fail complaining of a bad file descriptor (EBADF). Essentially, unicorn does some special handling to pass the file descriptors to the newly forked master process which then get dropped by bundler. You can definitely argue that we should just use rubygems-bundler, but I figured I'd bring up the discussion anyway.

Here's a simpler example:

# test.rb
if ARGV[0]
  puts "Opening #{ARGV[0]}"
  io = IO.for_fd(ARGV[0].to_i)
  puts "IO: #{io.inspect}"
else
  require 'tempfile'
  io = Tempfile.new("test")

  fork do
    args = %W{bundle exec ruby test.rb #{io.to_i}}
    args << { io.to_i => io } if RUBY_VERSION >= '2.0'
    exec(*args)
  end

  while true; end
end

$ touch Gemfile
$ rvm use 1.9.3
$ ruby test.rb
Opening 5
IO: #<IO:fd 5>
^C
$ rvm use 2.0
$ ruby test.rb
Opening 7
test.rb:3:in `for_fd': Bad file descriptor (Errno::EBADF)
    from test.rb:3:in `<main>'

If you change the exec line to just:

args = %W{ruby test.rb #{io.to_i}}

or set :close_others => false in bundler/cli.rb then the output is correct.

You can see the documentation changes between 2.0 and 1.9 in Process#spawn: 2.0 1.9.3

[gnufied]

After thinking about this for awhile, I think correct thing to do here is:

1. in 1.4 be backward compatible and stop bundle exec from closing file descriptors.
2. In next release though, we should move closer to Ruby 2.0 and enable bundle exec to close file descriptors by default, but provide a switch that user can pass which prevents bundler from closing the fds.

@indirect thoughts?

[indirect]

Well... as far as backwards compatibility goes, bundle exec still works the exact same way that it always has. Upgrading to Ruby 2.0 changes the semantics of exec, and that's not Bundler's doing, that's Ruby's. I'm not sure it's reasonable to expect Bundler to actively undo announced and breaking changes that are part of the Ruby interpreter you choose to run on. :/

That said, I think a flag to exec so that you can at least get the 1.9 behaviour on 2.0 is a good idea.

[xaviershay]

PR was merged.

[matthewd]

@indirect what are my chances of convincing you this should in fact be the default behaviour?

My argument is basically:

• ruby's default changed to protect the developer of the execing program from accidentally leaking its own FDs
• bundler doesn't leave FDs of its own lying around to potentially leak
• 'bundle exec' is designed to "enhance" the execution environment of the process it's spawning
• if whoever execed us chose to pass the FDs in, it's reasonable to assume they should be passed on: they're not ours to protect

[indirect]

I am happy to change it in the 2-0-dev branch for release in 2.0. We can't break backwards compatibility before then though.

On Wed, Jun 18, 2014 at 12:20 AM, Matthew Draper notifications@github.com wrote:

@indirect what are my chances of convincing you this should in fact be the default behaviour? My argument is basically:

• ruby's default changed to protect the developer of the execing program from accidentally leaking its own FDs
• bundler doesn't leave FDs of its own lying around to potentially leak
• 'bundle exec' is designed to "enhance" the execution environment of the process it's spawning

* if whoever execed us chose to pass the FDs in, it's reasonable to assume they should be passed on: they're not ours to protect

Reply to this email directly or view it on GitHub: https://github.com/bundler/bundler/issues/2628#issuecomment-46403465