Project

General

Profile

Actions

Bug #6230

closed

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

Added by nobuoka (yu nobuoka) over 12 years ago. Updated over 12 years ago.

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

Description

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

Updated by nobuoka (yu nobuoka) over 12 years ago

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

https://gist.github.com/2263660

Updated by naruse (Yui NARUSE) over 12 years ago

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

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

Actions #3

Updated by nobuoka (yu nobuoka) over 12 years ago

指摘ありがとうございます。 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 などする必要がなさそうだということで特になにもしていないのですが、これで大丈夫でしょうか?
(現在の実装では大丈夫そうですが、仕様に明記されていないことに期待していいのかどうか。。)

Updated by znz (Kazuhiro NISHIYAMA) over 12 years ago

nobuoka (yu nobuoka) wrote:

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

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

Actions #5

Updated by naruse (Yui NARUSE) over 12 years ago

  • 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]

Updated by naruse (Yui NARUSE) over 12 years ago

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

znz (Kazuhiro NISHIYAMA) wrote:

nobuoka (yu nobuoka) wrote:

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

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

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

Updated by nobuoka (yu nobuoka) over 12 years ago

znz さん、naruse さん、

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

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0