New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process.spawn with redirect causes ConcurrencyError #3038

Closed
djberg96 opened this Issue Jun 11, 2015 · 4 comments

Comments

Projects
None yet
3 participants
@djberg96
Contributor

djberg96 commented Jun 11, 2015

jruby 9.0.0.0.rc1 (2.2.2) 2015-06-10 a0bf3b3 Java HotSpot(TM) 64-Bit Server VM 25.45-b02 on 1.8.0_45-b14 +jit [darwin-x86_64]
Yosemite
java version "1.8.0_45"
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)

It seems JRuby does not like redirecting output on Process.spawn:

# Where test.rb is a very simple puts "hello" kind of script
Process.spawn("test.rb", :out => ['test.log', 'a'])

# Result
ConcurrencyError: Detected invalid array contents due to unsynchronized modifications with concurrent users
  spawn at org/jruby/RubyProcess.java:1295
@bbrowning

This comment has been minimized.

Show comment
Hide comment
@bbrowning

bbrowning Jun 11, 2015

Contributor

The :out key's array can have 1, 2, or 3 values in it. This ConcurrencyError is actually thrown because of an ArrayIndexOutOfBoundsException thrown at https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/RubyArray.java#L4398. That gets called via https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/util/io/PopenExecutor.java#L1537. It looks like the code here and on line 1528 of PopenExecutor assumes that requesting an array index past the end of the array will return nil. But, what's happening instead is this ConcurrencyError gets thrown.

The code needs to either check array length before accessing elements or use another method besides eltOk that returns nil instead of throwing.

Contributor

bbrowning commented Jun 11, 2015

The :out key's array can have 1, 2, or 3 values in it. This ConcurrencyError is actually thrown because of an ArrayIndexOutOfBoundsException thrown at https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/RubyArray.java#L4398. That gets called via https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/util/io/PopenExecutor.java#L1537. It looks like the code here and on line 1528 of PopenExecutor assumes that requesting an array index past the end of the array will return nil. But, what's happening instead is this ConcurrencyError gets thrown.

The code needs to either check array length before accessing elements or use another method besides eltOk that returns nil instead of throwing.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jun 15, 2015

Member

@bbrowning Ahh nice investigation. Simple enough to fix; I'm sure this was a flaw in my port, where I used a different method from MRI (i.e. one that errors rather than returning nil).

Member

headius commented Jun 15, 2015

@bbrowning Ahh nice investigation. Simple enough to fix; I'm sure this was a flaw in my port, where I used a different method from MRI (i.e. one that errors rather than returning nil).

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jun 15, 2015

Member

I added a spec for this behavior, but it appears JRuby is having trouble running these redirecting specs within RubySpec. We'll have to address that separately, but the reported spawn form should be working properly now.

Member

headius commented Jun 15, 2015

I added a spec for this behavior, but it appears JRuby is having trouble running these redirecting specs within RubySpec. We'll have to address that separately, but the reported spawn form should be working properly now.

@headius headius added this to the JRuby 9.0.0.0.rc2 milestone Jun 15, 2015

@djberg96

This comment has been minimized.

Show comment
Hide comment
@djberg96

djberg96 Jun 15, 2015

Contributor

Excellent, thank you!

Contributor

djberg96 commented Jun 15, 2015

Excellent, thank you!

headius added a commit to ruby/spec that referenced this issue Jun 29, 2015

seanpdoyle added a commit to thoughtbot/ember-cli-rails that referenced this issue Nov 17, 2015

Add `jruby-9000` to testing matrix
Closes [#241].

Redirects `STDERR` to `STDOUT` via the `STDOUT` file descriptor (as
described in the [`Kernel#spawn`][spawn] documentation).

Related to [jruby/jruby#3038].

Covered by [ruby/rubyspec].

[#241]: #241
[spawn]: http://ruby-doc.org/core-2.2.3/Kernel.html#method-i-spawn
[jruby/jruby#3038]: jruby/jruby#3038
[ruby/rubyspec]: https://github.com/ruby/rubyspec/blob/bfc40c3d73cc822e436b69b20967cf9b0eb7a4d8/shared/process/spawn.rb#L363-L369

seanpdoyle added a commit to thoughtbot/ember-cli-rails that referenced this issue Nov 17, 2015

Add `jruby-9000` to testing matrix
Closes [#241].

Redirects `STDERR` to `STDOUT` via the `STDOUT` file descriptor (as
described in the [`Kernel#spawn`][spawn] documentation).

Related to [jruby/jruby#3038].

Covered by [ruby/rubyspec].

[#241]: #241
[spawn]: http://ruby-doc.org/core-2.2.3/Kernel.html#method-i-spawn
[jruby/jruby#3038]: jruby/jruby#3038
[ruby/rubyspec]: https://github.com/ruby/rubyspec/blob/bfc40c3d73cc822e436b69b20967cf9b0eb7a4d8/shared/process/spawn.rb#L363-L369

seanpdoyle added a commit to thoughtbot/ember-cli-rails that referenced this issue Nov 19, 2015

Add `jruby-9000` to testing matrix
Closes [#241].

No longer redirects `STDERR` to `STDOUT` via the `STDOUT` file
descriptor (as described in the [`Kernel#spawn`][spawn] documentation).

Related to [jruby/jruby#3038].

Uses `poltergeist` instead of `capybara-webkit`.

Unfortunately, testing for `jruby` support is impossible until
[thoughtbot/capybara-webkit#725][#725] is resolved.

[#241]: #241
[spawn]: http://ruby-doc.org/core-2.2.3/Kernel.html#method-i-spawn
[jruby/jruby#3038]: jruby/jruby#3038
[#725]: thoughtbot/capybara-webkit#725

seanpdoyle added a commit to thoughtbot/ember-cli-rails that referenced this issue Nov 19, 2015

Add `jruby-9000` to testing matrix
Closes [#241].

No longer redirects `STDERR` to `STDOUT` via the `STDOUT` file
descriptor (as described in the [`Kernel#spawn`][spawn] documentation).

Related to [jruby/jruby#3038].

Uses `poltergeist` instead of `capybara-webkit`.

Unfortunately, testing for `jruby` support is impossible until
[thoughtbot/capybara-webkit#725][#725] is resolved.

[#241]: #241
[spawn]: http://ruby-doc.org/core-2.2.3/Kernel.html#method-i-spawn
[jruby/jruby#3038]: jruby/jruby#3038
[#725]: thoughtbot/capybara-webkit#725
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment