Skip to content
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.

Sign up for GitHub

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

Jump to bottom

IO.copy_stream with Zlib::GZipWriter broken in 9.1.3.0 and up #4202

Closed
ewr opened this issue Oct 4, 2016 · 5 comments
Closed

IO.copy_stream with Zlib::GZipWriter broken in 9.1.3.0 and up #4202

ewr opened this issue Oct 4, 2016 · 5 comments
Labels
JRuby 9000 stdlib
Milestone
JRuby 9.1.6.0

Comments

@ewr
Copy link

ewr commented Oct 4, 2016

In 9.1.3.0 and above, calling IO.copy_stream with a Zlib::GzipWriter breaks due to ArgumentError: wrong number of arguments callingrespond_to?(2 for 1).

I suspect it may have come from 9167f3a.

Reproduction script:

require 'tempfile'
require 'zlib'
require 'pry'

infile = Tempfile.new("copy_stream")
infile << "Test content"

outfile = Tempfile.new("copy_stream")

Zlib::GzipWriter.open(outfile.path) do |gz|
  IO.copy_stream(infile.path, gz)
end

Works on 9.0.4.0 and 9.1.2.0. Fails on 9.1.3.0, 9.1.4.0 and 9.1.5.0.
@headius
Copy link
Member

headius commented Oct 5, 2016

@ewr Good theory. I don't suppose you've bisected it?

I'll investigate.

@ewr
Copy link
Author

ewr commented Oct 5, 2016

Nope. Just wandered around looking for respond_to, and noticed that change occurred after the release where things broke.

@headius
Copy link
Member

headius commented Oct 5, 2016

Oh yeah, this is definitely from that commit. Simple fix coming.

This affects any copy_stream cases that need to coerce the streams. Great find and great investigation, thanks!

@headius
Copy link
Member

headius commented Oct 5, 2016

For future reference, you can narrow down an exception even more by passing -Xbacktrace.style=full to JRuby, which will include all the hidden Java stack frames. Here's the top of the error, with that flag set:

ArgumentError: wrong number of arguments calling `respond_to?` (2 for 1)
             getStackTrace at java/lang/Thread.java:1552
          getBacktraceData at org/jruby/runtime/backtrace/TraceType.java:246
              getBacktrace at org/jruby/runtime/backtrace/TraceType.java:47
          prepareBacktrace at org/jruby/RubyException.java:236
                  preRaise at org/jruby/exceptions/RaiseException.java:216
                  preRaise at org/jruby/exceptions/RaiseException.java:183
                    <init> at org/jruby/exceptions/RaiseException.java:111
         newRaiseException at org/jruby/Ruby.java:4176
          newArgumentError at org/jruby/Ruby.java:3569
        raiseArgumentError at org/jruby/runtime/Arity.java:277
        raiseArgumentError at org/jruby/internal/runtime/methods/JavaMethod.java:270
                      call at org/jruby/internal/runtime/methods/JavaMethod.java:1072
                      call at org/jruby/internal/runtime/methods/JavaMethod.java:724
                      call at org/jruby/internal/runtime/methods/DynamicMethod.java:205
              cacheAndCall at org/jruby/runtime/callsite/RespondToCallSite.java:161
                      call at org/jruby/runtime/callsite/CachingCallSite.java:195
                respondsTo at org/jruby/runtime/callsite/RespondToCallSite.java:101
               copy_stream at org/jruby/RubyIO.java:4075
                      call at org/jruby/RubyIO$INVOKER$s$0$2$copy_stream.gen:-1
                      call at org/jruby/internal/runtime/methods/JavaMethod.java:724
                      call at org/jruby/internal/runtime/methods/DynamicMethod.java:205
              cacheAndCall at org/jruby/runtime/callsite/CachingCallSite.java:358
                      call at org/jruby/runtime/callsite/CachingCallSite.java:195
  invokeOther1:copy_stream at blah.rb:11
          block in blah.rb at blah.rb:11

@headius headius added this to the JRuby 9.1.6.0 milestone Oct 6, 2016
@headius headius closed this as completed in 825877f Oct 6, 2016
headius added a commit that referenced this issue Oct 6, 2016
@headius
Fix regression spec for #4202.
8a4a945
@headius
Copy link
Member

headius commented Oct 6, 2016

So the actual bug here was that our GzipFile subclasses – GzipReader and GzipWriter – define their own respond_to? implementation, which did not support arity-2 calls (passing in the "include private" argument). I added that path, moved the code up from the child classes into GzipFile, and added a regression spec for this behavior.

MRI does not do this. Instead of having a path method defined that delegates to the underlying IO, they define a new method on every instance as a singleton method. We obviously don't want to do that, so we define path always and delegate respond_to? :path to the underlying IO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
JRuby 9000 stdlib
Projects
None yet
Milestone
JRuby 9.1.6.0
Development

No branches or pull requests
2 participants
@headius @ewr