Rails 4, streaming blocked by `@body.respond_to?(:body_parts)`

[kevinmtrowbridge]

In the response.rb#write_body method, there's this line (in the if / elseif block):

elsif @body.respond_to?(:body_parts)

This was added by this commit: https://github.com/jruby/jruby-rack/commit/7b87c78f807140bfb86e0b7054d6a967ac02301a

... with the commit message:

with Rails 3.x ... thus check for body_parts on the body object

... we use raw data streaming (via passing an enumerator to reponse_body) with jruby-rack & Jetty. The streaming worked fine with Rails 3. About a year ago we upgraded to Rails 4. This introduced a subtle bug in that, the streaming no longer worked, but it did not fail outright, what happened is that it fell back to behavior where it would buffer the entire response body in memory, then send it to the client when done.

In many cases this was ok, however, we're streaming the results of an SQL query to the client, so it could result in arbitrarily huge files. In some cases, it was essentially behaving as a massive memory leak which could crash the application server.

I dug into it and found eventually that the line @body.respond_to?(:body_parts) blocks the write_body method. body_parts method in Rails 4 looks like so:

def body_parts
  parts = []
  @stream.each { |x| parts << x }
  parts
end

I can only assume that the body_parts method hangs out there till the entire response body has been streamed, blocking the flushing of the response. If I comment out that block, ie, make it look like this:

      def write_body(response)
        body = nil
        begin
          if @body.respond_to?(:call) && ! @body.respond_to?(:each)
            @body.call response.getOutputStream
          elsif @body.respond_to?(:to_path) # send_file
            send_file @body.to_path, response
          elsif @body.respond_to?(:to_channel) &&
              ! object_polluted_with_anyio?(@body, :to_channel)
            body = @body.to_channel # so that we close the channel
            transfer_channel body, response.getOutputStream
          elsif @body.respond_to?(:to_inputstream) &&
              ! object_polluted_with_anyio?(@body, :to_inputstream)
            body = @body.to_inputstream # so that we close the stream
            body = Channels.newChannel(body) # closing the channel closes the stream
            transfer_channel body, response.getOutputStream
          # elsif @body.respond_to?(:body_parts) && @body.body_parts.respond_to?(:to_channel) &&
          #     ! object_polluted_with_anyio?(@body.body_parts, :to_channel)
          #   # ActionDispatch::Response "raw" body access in case it's a File
          #   body = @body.body_parts.to_channel # so that we close the channel
          #   transfer_channel body, response.getOutputStream
          else
            if dechunk?
              write_body_dechunked response.getOutputStream
            else
              output_stream = response.getOutputStream
              # 1.8 has a String#each method but 1.9 does not :
              method = @body.respond_to?(:each_line) ? :each_line : :each
              @body.send(method) do |line|
                output_stream.write(line.to_java_bytes)
                output_stream.flush if flush?
              end
            end
          end
        rescue LocalJumpError
          # HACK: deal with objects that don't comply with Rack specification
          @body = [ @body.to_s ]
          retry
        rescue java.io.IOException => e
          raise e if ! client_abort_exception?(e) || ! self.class.swallow_client_abort?
        ensure
          @body.close if @body.respond_to?(:close)
          body && body.close rescue nil
        end
      end

... as I said if I comment out that part, the streaming starts working again.

The commit message mentioned this fix being for Rails 3, so I am hoping this part is not necessary for Rails 4. I will report back if our QA process finds any issues with this change.

[kares]

the idea here was to do efficient streaming on the native side ... seems that its not that easy starting Rails 4.0 as the framework assumes it does now the best way to stream data. we tried to acess the very raw body here:

  def body_parts
    @body
  end

https://github.com/rails/rails/blob/3-2-stable/actionpack/lib/action_dispatch/http/response.rb#L134

since ActionPack 4.0 this is now wrapped in a "stream" object that also handles response callbacks :

https://github.com/rails/rails/blob/4-0-stable/actionpack/lib/action_dispatch/http/response.rb#L202

  def body=(body)
    @blank = true if body == EMPTY

    if body.respond_to?(:to_path)
      @stream = body
    else
      synchronize do
        @stream = build_buffer self, munge_body_object(body)
      end
    end
  end

  def body_parts
    parts = []
    @stream.each { |x| parts << x }
    parts
  end

.. body_parts simply reads it all as you observed

while your work-around is fine it might perform less optimally than under 3.x e.g. when serving files. due the callbacks this needs more investigation whether we can stream on or own, the quick fix is going to be checking respond_to?(:stream) and if so not using the path you have commented ...

[kares]

@kevinmtrowbridge if you're able to please verify that the patch added works fine ... will try to add a test for the case but a real-world confirmation is always nice, thanks!