Bug #6230: [WEBrick] WEBrick::HTTPResponse#body の IO オブジェクトの読み込みに read メソッドを使っているため必要以上にブロックされる

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 1.9.3p125 (2012-02-16 revision 34643) [x86_64-linux]

[nobuoka (yu nobuoka)]

WEBrick::HTTPResponse の @body には IO オブジェクトを設定できますが、@body に設定された IO オブジェクトからの読み出しの際に IO#read( @buffer_size ) で行われるため、@buffer_size よりも小さなデータを定期的に送りたい場合などに、必要以上にブロックされてしまいます。 IO#read メソッドの代わりに IO#readpartial メソッドを使用するとよいかと思うのですがどうでしょうか。

patch を添付します。

diff --git a/lib/webrick/httpresponse.rb b/lib/webrick/httpresponse.rb
index 0d36c07..4942588 100644
--- a/lib/webrick/httpresponse.rb
+++ b/lib/webrick/httpresponse.rb
@@ -330,13 +330,17 @@ module WEBrick
if @request_method == "HEAD"

do nothing

elsif chunked?

•      while buf = @body.read(@buffer_size)
  

•        next if buf.empty?
  

•        data = ""
  

•        data << format("%x", buf.bytesize) << CRLF
  

•        data << buf << CRLF
  

•        _write_data(socket, data)
  

•        @sent_size += buf.bytesize
  

•      begin
  

•        while true
  

•          buf = @body.readpartial( @buffer_size )
  

•          next if buf.empty?
  

•          data = ""
  

•          data << format("%x", buf.bytesize) << CRLF
  

•          data << buf << CRLF
  

•          _write_data(socket, data)
  

•          @sent_size += buf.bytesize
  

•        end
  

•      resuce EOFError # do nothing
       end
       _write_data(socket, "0#{CRLF}#{CRLF}")
     else
  

具体的に困る状況は、例えば以下のように Server-Sent Events で応答するサーバーを実現するような場合です。

require 'webrick'

server = WEBrick::HTTPServer.new( Port: 8000 )
server.mount_proc( '/time_stream' ) do |req, res|
res.content_type = 'text/event-stream'
r,w = IO.pipe
res.body = r
res.chunked = true
t = Thread.new do
10.times do
Thread.pass
w << 'data: ' << Time.now.to_s << "\x0D\x0A"
w << "\x0D\x0A"
sleep 1
end
w.close()
end
end

trap :INT do server.shutdown end
server.start

[nobuoka (yu nobuoka)]

patch のインデント崩れと綴り間違い (rescue) に気づいたため、gist にあげました。

https://gist.github.com/2263660

[naruse (Yui NARUSE)]

方向としては良いかと思います。

readpartial って、@buffer_size が 0 でない限り、空文字列を返すことってないような気がします。
test/webrick/test_httpserver.rb 用のテストもお願いします。

[nobuoka (yu nobuoka)]

指摘ありがとうございます。 readpartial は長さ 1 以上の String オブジェクトを返す、という仕様になっているということを確認したので、空文字列かどうかで分岐する部分は削除しました。 また、毎回 buf を生成するのは非効率的だと思ったので、buf をループ内で再利用するようにしました。
また、test/webrick/test_httpserver.rb には元々 chunked = true の状態でのテストが無かったので、test_response_io_without_chunked_set メソッドを元に、chunked = true とした場合のテストを追加しました。

patch は gist にあげました:
https://gist.github.com/2263660/14abe871c1446d4f96285668ac2f49fa284489b5

[疑問点]
現状の動作を見たところ、readpartial の第 2 引数に空文字列でない String オブジェクトを渡しても、元々格納されていたデータは readpartial メソッド内で削除されているようでしたので、とりあえず buf.clear などする必要がなさそうだということで特になにもしていないのですが、これで大丈夫でしょうか?
(現在の実装では大丈夫そうですが、仕様に明記されていないことに期待していいのかどうか。。)

[znz (Kazuhiro NISHIYAMA)]

nobuoka (yu nobuoka) wrote:

(現在の実装では大丈夫そうですが、仕様に明記されていないことに期待していいのかどうか。。)

そういう場合はその期待が変わったときに気づいて対処出来るように、
期待している挙動のテストを追加しておくと良いのではないでしょうか。

[naruse (Yui NARUSE)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r35253.
yu, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• lib/webrick/httpresponse.rb (WEBrick::HTTPResponse#send_body_io):
  use readpartial to get data even if the response is streaming data and
  each data is smaller than @buffer_size.
  patched by yu nobuoka. [ruby-dev:45471] [Bug #6230]

[naruse (Yui NARUSE)]

r35253 でマージしました、ありがとうございました。

znz (Kazuhiro NISHIYAMA) wrote:

nobuoka (yu nobuoka) wrote:

(現在の実装では大丈夫そうですが、仕様に明記されていないことに期待していいのかどうか。。)

そういう場合はその期待が変わったときに気づいて対処出来るように、
期待している挙動のテストを追加しておくと良いのではないでしょうか。

そうですね、そのようなテストを追加するのが良いかと思います。
rdoc も追加した方が良いかな。

[nobuoka (yu nobuoka)]

znz さん、naruse さん、

なるほど、確かにそうするのが良いですね。
ありがとうございます。