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.

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

IO.to_outputstream (a.k.a. RubyIO.getOutStream) returns a non-writable stream in some cases. #3101

Closed
raelik opened this issue Jul 2, 2015 · 9 comments

Comments

@raelik
Copy link

raelik commented Jul 2, 2015

While doing some testing with chunked responses on a plain Rack app running under puma 2.11.3 (using rack.hijack to get access to the raw socket) on jruby 1.7.20.1, I was getting a "not opened for writing" IOException, which I traced back to checkWritable() in the ChannelStream class used internally by RubyIO. The OutputStreamAdapter ultimately returned by getOutStream() uses checkWritable() in many places, so I was able to get around this issue by using getChannel() instead and wrapping it in my own OutputStream using Channels.newOutputStream(). This issue has been resolved in 9k via the overhaul of the IO system.

@headius
Copy link
Member

headius commented Jul 2, 2015

So was it returning an OutputStream wrapping a read-only channel? Channels.toOutputStream should be doing the right thing...

@headius
Copy link
Member

headius commented Jul 2, 2015

Ok, had to refresh my memory a bit.

I see that we create our own OutputStreamAdapter that provides all the OutputStream behavior. This calls back into ChannelStream, which checks whether the stream was opened with write mode. If the IO is not set up for write, why are you trying to get an OutputStream from it? I'm a little confused...some example code might help. Or a patch, if you have an idea how you'd like to solve this!

@raelik
Copy link
Author

raelik commented Jul 2, 2015

The specific case I'm using it for is with the socket that puma passes in to a rack.hijack lambda to do chunked encoding. Here's a hello world example of a chunked response using rack.hijack:

def run
  output = "Hello World!"
  hijack = -> socket {
    socket.print(("%x\r\n" % output.bytesize) + output + "\r\n0\r\n\r\n")
  }
  [200, {'Transfer-Encoding' => 'chunked', 'rack.hijack' => hijack, nil]
end

Passing a lambda to the rack.hijack "header" tells rack that you want to do a partial hijack of the connection and gain full control of the client connection (writable). In my particular case, I'm iterating over a very large JDBC ResultSet and am streaming CSV back to the client in chunks. I've implemented this iteration and CSV conversion in Java for performance, and am using the Apache HTTP Commons library to handle the chunked encoding, using ChunkedOutputStream and the SessionOutputBufferImpl classes. So in my case, the lambda above looks more like this:

res = stmt.execute_query(sql)
-> socket { begin
  writer = ResultSetCsvWriter(socket.to_outputstream, CHUNK_SIZE)
  writer.write_all_rows_and_close(res)
ensure
  res.close
  stmt.close
  conn.close
end }

ResultSetCsvWriter is my Java class, with the following constructor:

  public ResultSetCsvWriter(OutputStream stream, int buffer_size) {
    int chunk_size = buffer_size / 4;
    int chunk_buffer = chunk_size + ((int) Math.floor(Math.log(chunk_size) / Math.log(16))) + 5;
    metrics = new HttpTransportMetricsImpl();
    buffer = new SessionOutputBufferImpl(metrics, chunk_buffer);
    buffer.bind(stream);
    chunked = new ChunkedOutputStream(chunk_size, buffer);
    writer = new BufferedWriter(new OutputStreamWriter(chunked), buffer_size);
  }

writeAllRowsAndClose does exactly what you would expect, iterating over the rows in the result set, and each column in those rows, writing it all to the BufferedWriter with interspersed commas and line feeds, closing the writer and ChunkedOutputStream when it's done.

@headius
Copy link
Member

headius commented Jul 3, 2015

My confusion is why you'd be seeing "not opened for writing" if you're using our OutputStreamAdapter against a channel opened for write. Or put differently...if it's opened for write, our adapter should work fine. If it's not opened for write, then how can any output work? What exactly is the problem with JRuby we need to fix?

@raelik
Copy link
Author

raelik commented Jul 7, 2015

I'm as similarly confused as you are, as that is exactly what seems to be happening. The channel is definitely opened for write, as I can use getChannel() and wrap it in my own OutputStream to write to it (or just write to it in Ruby). Something is failing with regards to the OutputStreamAdapter. Whether that failure has to do with the OutputStreamAdapter itself, or something that puma is doing to the socket before it calls the lambda which causes it to not be able to write to the socket, I don't know.

@headius
Copy link
Member

headius commented Jul 7, 2015

Can you set up a git repo that reproduces the issue for me?

jkutner pushed a commit to jkutner/jruby that referenced this issue Jul 10, 2015
ee9abb8 Fix spec title for a Module#name spec.
aa55755 Add spec for Enumerator.new form calling bare each.
f2d4242 Exclude the Europe/Lisbon time zone change on OS X as it seems inaccurate.
b0a5ebc Fix syntax for Ruby 2.0 version for Travis.
402e570 Require at least 2.0.0p598 on Travis.
b5fa5d3 Use a consistent style for literal arrays in Array#concat spec.
8f8fb81 rb_sym2str() is 2.2-only.
0e7a261 Fix core specs for 2.0.0p643.
92a1506 Fix file name for Process::Status#&.
1c55da4 Fix guard for keyword argument spec.
3b9ac9b Do not speculate on identity of keyword-like argument hashes.
b633b06 Adding missing require for FileUtils.
5531d57 Reintroduce Number arg specs for Regexp.new
1574a22 use already defined methods from the fixture class
c265a11 implement specs for comparing a method defined via define_method and a method defined via def
8448314 ported over jruby's fix for Proc#curry, this closes jruby#2951
914090e Removed timeout from Process.detach specs.
f7500c2 Added spec for Array#concat with shifted start.
31a833f Added stdarg.h include to ruby.h.
92d81b7 Added C-API specs for rb_vsprintf.
873b7aa Added RHASH_SET_IFNONE/rb_hash_set_ifnone
8a49b9c Added rb_sym2str()
bcb70e2 Added rb_hash_lookup2()
fb2bf67 Add specs for cases where timezone used was changed.
7bc8c1b Fixed testing Symbol is a valid constant.
ddd1f8f Add failing spec for prepended module method aliasing.
5d3ce50 Fix Module#public_instance_method for prepended modules.
fcacf19 Use consistent style in prepend specs.
76682f4 Add specs for prepended module method ownership (one failing).
9006ddb Ruby spec for multibyte constant names
104176f Add specs for #3360 (case-when with splatted arrays from variables).
48a30a6 Update Proc#curry specs to include 0-ary lambdas
e0c9464 Updated String#sub/sub! specs for ArgumentError
f3ed63e Enumerator#size specs for Enumerator
0230e29 Enumerator#size specs for Struct
e8cce95 Enumerator#size specs for IO
47a6d4a Enumerator#size specs for ENV
1d24cd9 Specs for Enumerator#size for Dir
459dd14 Specs for Enumerator#size for ARGF
e09c1d6 Updated Enumerator#size specs for String
561f896 Enumerator#size specs for Numeric#step.
3683eff Enumerator#size specs for Kernel#loop.
7f290d1 Update Enumerator#size specs for Integer
9c9a9ab Enumerator#size calculation specs for Range
d3eec29 Enumerator#size calculation specs for Hash
b1c468d New Enumerator#size tests for the Array class.
c9719f1 New Enumerator/Enumerable size calculation tests
61b9654 Extra arg validation tests on each_cons/each_slice
3f4351a Creates enumeratorized shared specs.
585fe5c Remove special case logic for String#slice(<Symbol>) It is okay to remove this special-case logic here because Symbol#to_int no longer exists in Ruby 1.9+, so Symbol-to-Fixnum coercion will fail with TypeError (which matches MRI behavior). Resolves jruby#3335.
a038ec6 refactored Marshal.load specs and updated CI tags
3a7876b Added new Marshal.load spec for object with user-defined _dump
fc96758 Added more Marshal.load specs for objects with user-defined _dumps
b1b521a Fix Array#slice! with length argument for single item arrays
123f35c Add Array#slice! positive length check and spec
c811934 Spec for calling top-level methods in wrapped load
579dc2c Updates spec name.
6079006 Adds specs to Array methods that return an Enumerator. The tests check if the size method on the resulting Enumerator instance returns the correct value.
bb14c39 Adds specs to Enumerable methods that return an Enumerator. The tests check if the size method on the resulting Enumerator instance returns the correct value.
ebf235b Adds NumerousWithSize class to help spec the size method behaviour on Enumerable methods that return an Enumerator.
6414afe Removing each_with_index changes to go into a separate pull request
15322d2 Updating spec name and method name per code review
894171d Fix Fixnum minus Bignum to not assume the result is a Bignum. Resolves jruby#3316
80748d9 Trying to fix each_slice and each_with_index
9dbff60 Added spec for simple Object#inspect result.
f227519 Added spec for Kernel#inspect.
50005d2 Failing spec for String#encode with Emacs-Mule.
f6ffd5f Added spec for a = b = m 1. Closes jruby#3266.
c8b23e5 Fixed Enumerable#flat_map specs.
282810a Added C-API spec for rb_iv_{set, get} with non-ivar name.
9d4866c Basic specs for optional variable assignments.
1adf79a Nuke NetBeans config files.
fba96b3 More specs for String#ascii_only? + String#replace
a214db3 Add failing spec for distinguishing between a vcall and method when missing. (Missing method => NoMethodError, missing vcall => NameError) Demonstrates issue jruby#3101.
f538eb5 Re-removed obsoleted specs.
e6d17ea Added specs for String#scrub with ASCII/BINARY.
657dcdf Added more keyword specs.
f629e3f Added spawn specs for STD{IN, OUT, ERR}.
b4121d8 Add flat_map spec for enumerables that yield multiple values
0edd4ca Adding a missing 'should'
caf25d4 Rely on data file instead of signal file and always write to it.
bcf1fe2 Stop waiting for the daemon as soon as we see the signal file.

git-subtree-dir: spec/ruby
git-subtree-split: ee9abb86f494bb0aac0d1cb6300e48c82c1d087c
@headius
Copy link
Member

headius commented Aug 18, 2015

I'm wondering if this might be the same issue as #3189, where a stream was getting closed for both read and write when requesting it be closed just for write. Are you on Windows?

@raelik
Copy link
Author

raelik commented Aug 18, 2015

No, this was on Linux. Sorry I haven't set up a repo/gist to reproduce the issue, been caught up in work fires.

@enebo enebo added this to the Won't Fix milestone May 16, 2017
@enebo
Copy link
Member

enebo commented May 16, 2017

1.7.x is EOL and this will not get fixed. Marking WONTFIX

@enebo enebo closed this as completed May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants