Details

    • Type: Improvement Improvement
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: JRuby 1.7.0.pre1
    • Component/s: None
    • Labels:
      None
    • Environment:
      jruby 1.5.2 (ruby 1.8.7 patchlevel 249) (2010-08-20 1c5e29d) (Java HotSpot(TM) Client VM 1.6.0_18) [x86-java]
    • Testcase included:
      yes
    • Number of attachments :
      0

      Description

      Currently if you run a system call attempting to call jruby, you can get mysterious, unexpected results.

      ex:

      >> system("jruby --server -e 3")
      jruby: unknown option --server
      => false

      However, in reality there is not even a jruby.exe in my path, so this is a doubly mistaken error.

      Also it can cause problems when testing, etc:

      http://groups.google.com/group/jruby-users/browse_thread/thread/9690d3f4027a9ea6
      http://www.ruby-forum.com/topic/204508

      Since this is an "optimization" to make things faster in specific instances, it is surprising in other instances, so this is a request to turn it off by default.

      Thanks!
      -r

        Activity

        Hide
        Roger Pack added a comment - - edited

        Having it default to true also prevents one from, by default, being able to use multiple sub-processes to run jruby (this is what got me, today).

        1. this code runs differently, which is a gotcha with it on by default:

        (0..20).map{|n| Thread.new

        { system("jruby -e 'sleep 10'") }

        }.each

        {|th| th.join}

        Also, it appears others have shared this opinion:

        http://www.mail-archive.com/dev@jruby.codehaus.org/msg02874.html

        Show
        Roger Pack added a comment - - edited Having it default to true also prevents one from, by default, being able to use multiple sub-processes to run jruby (this is what got me, today). this code runs differently, which is a gotcha with it on by default: (0..20).map{|n| Thread.new { system("jruby -e 'sleep 10'") } }.each {|th| th.join} Also, it appears others have shared this opinion: http://www.mail-archive.com/dev@jruby.codehaus.org/msg02874.html
        Hide
        Hiro Asari added a comment -

        If we are to change the default value, 1.6 is a good time to do it (as opposed to, say 1.6.1).

        Any comment?

        Show
        Hiro Asari added a comment - If we are to change the default value, 1.6 is a good time to do it (as opposed to, say 1.6.1). Any comment?
        Hide
        Roger Pack added a comment -

        If charles' proposal for Kernel#ruby is accepted then the default should be true still? Of course, until then I think it should default to false, so +1
        -r

        Show
        Roger Pack added a comment - If charles' proposal for Kernel#ruby is accepted then the default should be true still? Of course, until then I think it should default to false, so +1 -r
        Hide
        Roger Pack added a comment -

        Looks like the concensus from http://jira.codehaus.org/browse/JRUBY-1579 was also that it should default to false, to avoid subtle bugs. Thanks!

        Show
        Roger Pack added a comment - Looks like the concensus from http://jira.codehaus.org/browse/JRUBY-1579 was also that it should default to false, to avoid subtle bugs. Thanks!
        Hide
        Roger Pack added a comment -

        Another possible appearance: http://www.ruby-forum.com/topic/1019579

        It seems to just confuse, it appears to me...

        Show
        Roger Pack added a comment - Another possible appearance: http://www.ruby-forum.com/topic/1019579 It seems to just confuse, it appears to me...
        Hide
        Charles Oliver Nutter added a comment -

        Well there's a bigger problem here: Most Ruby libraries still do "exec" or "system" or `` using "ruby" as the command to run. That's obviously a bug in those libraries; any ruby install with a command named something other than "ruby" will fail to work properly. But what can we do?

        Currently we check for "ruby" at the start of the command and then attempt to parse out the arguments and run it in-process. That keeps it in JRuby, but has the side effect of not actually running a separate process nor running "ruby", if that was the actual intent. If we turned off inproc launching completely, a system('ruby ...') call would suddenly stop running in JRuby. We could again mine out the "ruby" part and make it "jruby", but the parsing is more complicated and we'd be explicitly preventing "ruby" from being run as an external command from within JRuby.

        So you see, it's actually a very sticky problem. The inproc launching, for all its faults, still seems like the cleanest way to address all the following issues:

        • Commands being executed as "ruby" that are intended to use the currently-executing ruby runtime (i.e. not using 'jruby' command on jruby, etc)
        • Many libraries and apps shelling out frequently (such as for running tests or specs) penalizing us for our slow startup if we launch a whole new JVM

        I appreciate that it's confusing, but there are many challenges in changing it.

        Show
        Charles Oliver Nutter added a comment - Well there's a bigger problem here: Most Ruby libraries still do "exec" or "system" or `` using "ruby" as the command to run. That's obviously a bug in those libraries; any ruby install with a command named something other than "ruby" will fail to work properly. But what can we do? Currently we check for "ruby" at the start of the command and then attempt to parse out the arguments and run it in-process. That keeps it in JRuby, but has the side effect of not actually running a separate process nor running "ruby", if that was the actual intent. If we turned off inproc launching completely, a system('ruby ...') call would suddenly stop running in JRuby. We could again mine out the "ruby" part and make it "jruby", but the parsing is more complicated and we'd be explicitly preventing "ruby" from being run as an external command from within JRuby. So you see, it's actually a very sticky problem. The inproc launching, for all its faults, still seems like the cleanest way to address all the following issues: Commands being executed as "ruby" that are intended to use the currently-executing ruby runtime (i.e. not using 'jruby' command on jruby, etc) Many libraries and apps shelling out frequently (such as for running tests or specs) penalizing us for our slow startup if we launch a whole new JVM I appreciate that it's confusing, but there are many challenges in changing it.
        Hide
        Roger Pack added a comment -

        Seems there are two things at play
        1) it's faster to do it in process
        2) it's easier on the end user to grab "ruby" command lines and parse them into jruby.

        Perhaps the default behavior should be to grab "ruby" command lines and then start a whole new process using java.exe again, with correct arguments (i.e. basically just replacing "ruby" with Gem.ruby)? I'd guess the only drawback to this is the slowdown, but users could notice and avoid that by passing some other (optional) parameter like inproc or what not.

        Then it would be satisfying number 2, but number 1's side effects are also eliminated?

        Show
        Roger Pack added a comment - Seems there are two things at play 1) it's faster to do it in process 2) it's easier on the end user to grab "ruby" command lines and parse them into jruby. Perhaps the default behavior should be to grab "ruby" command lines and then start a whole new process using java.exe again, with correct arguments (i.e. basically just replacing "ruby" with Gem.ruby)? I'd guess the only drawback to this is the slowdown, but users could notice and avoid that by passing some other (optional) parameter like inproc or what not. Then it would be satisfying number 2, but number 1's side effects are also eliminated?
        Hide
        Charles Oliver Nutter added a comment -

        For JRuby 1.7, we finally pulled the trigger and made this change.

        Show
        Charles Oliver Nutter added a comment - For JRuby 1.7, we finally pulled the trigger and made this change.

          People

          • Assignee:
            Charles Oliver Nutter
            Reporter:
            Roger Pack
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: