Bug #5237
closedIO.copy_stream calls #read on an object infinitely many times
Description
The code for IO.copy_stream appears to support passing any object that implements #readpartial or #read. For example, passing an instance of StringIO works just fine:
sasha:rubinius2.0 brian$ irb
ruby-1.9.2-p290 :001 > puts RUBY_DESCRIPTION
ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin10.8.0]
=> nil
ruby-1.9.2-p290 :002 > require 'stringio'
=> true
ruby-1.9.2-p290 :003 > s = StringIO.new "this is a test"
=> #StringIO:0x00000100851190
ruby-1.9.2-p290 :004 > IO.copy_stream s, "copy_stream_stringio"
=> 14
ruby-1.9.2-p290 :005 > IO.read "copy_stream_stringio"
=> "this is a test"
ruby-1.9.2-p290 :006 >
However, passing an object that implements eg #read according to the specification of IO#read, causes IO.copy_stream to endlessly call #read on the object:
class A
def initialize
@count = 0
end
def read(bytes, buffer)
print "read: #{@count += 1}\r"
buffer.replace "this is a test"
return nil
end
end
a = A.new
IO.copy_stream a, "copy_stream_test"
ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin10.8.0]
copy_stream_bug.rb:7:in write': Interrupt from copy_stream_bug.rb:7:in
print'
from copy_stream_bug.rb:7:in read' from copy_stream_bug.rb:14:in
copy_stream'
from copy_stream_bug.rb:14:in `'
Thanks,
Brian
Files
Updated by normalperson (Eric Wong) over 12 years ago
- File 0001-avoid-infinite-loop-of-IO.copy_stream.patch 0001-avoid-infinite-loop-of-IO.copy_stream.patch added
The class that implements an IO#read-like method without clearing
the destination buffer on EOF is arguably broken, but infinite
looping is bad, I think.
The attached simple patch should fix the issue. Also available via git:
git pull git://bogomips.org/ruby.git copy-stream-infinite-loop
Updated by brixen (Brian Shirai) over 12 years ago
On Sat, Aug 27, 2011 at 3:54 AM, Eric Wong normalperson@yhbt.net wrote:
Issue #5237 has been updated by Eric Wong.
File 0001-avoid-infinite-loop-of-IO.copy_stream.patch added
The class that implements an IO#read-like method without clearing
the destination buffer on EOF is arguably broken, but infinite
looping is bad, I think.
Ah, you are right, it really should replace buffer with "" (according
to the code) and return nil according to the docs. Seems more than a
little weird, however, to empty the buffer to signal EOF, especially
since the buffer argument is optional.
Cheers,
Brian
The attached simple patch should fix the issue. Â Also available via git:
 git pull git://bogomips.org/ruby.git copy-stream-infinite-loop
Bug #5237: IO.copy_stream calls #read on an object infinitely many times
http://redmine.ruby-lang.org/issues/5237Author: Brian Ford
Status: Open
Priority: Normal
Assignee:
Category: core
Target version:
ruby -v: ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin10.8.0]The code for IO.copy_stream appears to support passing any object that implements #readpartial or #read. For example, passing an instance of StringIO works just fine:
sasha:rubinius2.0 brian$ irb
ruby-1.9.2-p290 :001 > puts RUBY_DESCRIPTION
ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin10.8.0]
Â
Updated by kosaki (Motohiro KOSAKI) over 12 years ago
- ruby -v changed from ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin10.8.0] to -
The class that implements an IO#read-like method without clearing
the destination buffer on EOF is arguably broken, but infinite
looping is bad, I think.Ah, you are right, it really should replace buffer with "" (according
to the code) and return nil according to the docs. Seems more than a
little weird, however, to empty the buffer to signal EOF, especially
since the buffer argument is optional.
I'm curious. Which doc do you talk about?
Updated by brixen (Brian Shirai) over 12 years ago
On Sat, Aug 27, 2011 at 10:55 PM, KOSAKI Motohiro
kosaki.motohiro@gmail.com wrote:
The class that implements an IO#read-like method without clearing
the destination buffer on EOF is arguably broken, but infinite
looping is bad, I think.Ah, you are right, it really should replace buffer with "" (according
to the code) and return nil according to the docs. Seems more than a
little weird, however, to empty the buffer to signal EOF, especially
since the buffer argument is optional.I'm curious. Which doc do you talk about?
The RDoc, which refers to returning nil or "" depending on what
arguments are passed. For the case here, it does not matter what value
is returned. The only way to not loop infinitely is to set the buffer
to "".
Updated by kosaki (Motohiro KOSAKI) over 12 years ago
If you are reading following paragraph,
- At end of file, it returns
nil
or""
- depend on length.
ios.read()
andios.read(nil)
returns""
.ios.read(positive-integer)
returnsnil
.
you have to read following paragraph too, I think.
- If length is a positive integer,
- it try to read length bytes without any conversion (binary mode).
- It returns
nil
or a string whose length is 1 to length bytes.nil
means it met EOF at beginning.
AT BEGINNING
Thus, positive length destination buffer and returning nil are exclusive. Now, I don't think
we need doc fix.
but I'm not talking about a behavior change. I have no strong opinion. It's another story. I'm waiting akr-san's response.
Updated by brixen (Brian Shirai) over 12 years ago
Hi,
On Tue, Aug 30, 2011 at 8:48 PM, Motohiro KOSAKI
kosaki.motohiro@gmail.com wrote:
Issue #5237 has been updated by Motohiro KOSAKI.
If you are reading following paragraph,
- Â At end of file, it returns
nil
or""
- Â depend on length.
- Â
ios.read()
and- Â
ios.read(nil)
returns""
.- Â
ios.read(positive-integer)
returnsnil
.you have to read following paragraph too, I think.
- Â If length is a positive integer,
- Â it try to read length bytes without any conversion (binary mode).
- Â It returns
nil
or a string whose length is 1 to length bytes.- Â
nil
means it met EOF at beginning.AT BEGINNING
Thus, positive length destination buffer and returning nil are exclusive. Now, I don't think
we need doc fix.but I'm not talking about a behavior change. I have no strong opinion. It's another story. I'm waiting akr-san's response.
I'm not following your explanation. The return value, whether nil or
anything else, including an empty string or any string (one byte or
not) will NOT cause the copy_stream method to return. It will loop
infinitely unless the buffer that is passed in is set to "". In fact,
I can return anything from #read as long as I set the buffer to "".
This is bizarre.
At the least, the rdoc should note that to participate in this API,
#read must set the buffer to "" to signal EOF. Still, in my opinion,
that is a bad API. The return value should be the sole way to signal
EOF given that the arguments to #read are optional.
sasha:rubinius2.0 brian$ cat copy_stream_bug.rb
class A
def initialize
@count
Updated by akr (Akira Tanaka) over 12 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
This issue was solved with changeset r33515.
Brian, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
- io.c (copy_stream_fallback_body): check nil for EOF of read method.
patch by Eric Wong. [ruby-core:39134] [Bug #5237]