Bug #8669

outbuf can be "temporarily" locked forever in IO#read

Added by Masaki Matsushita 9 months ago. Updated 9 months ago.

[ruby-core:56121]
Status:Closed
Priority:Normal
Assignee:Masaki Matsushita
Category:core
Target version:2.1.0
ruby -v:ruby 2.1.0dev (2013-07-23 trunk 42132) [x86_64-linux] Backport:1.9.3: DONE, 2.0.0: DONE

Description

Following code make outbuf "temporarily" locked forever.
It is needed to ensure rbstrunlocktmp().

str = ""
t = Thread.new(str) do |str|
r, = IO.pipe
r.read(nil, str)
end
sleep 1
t.raise
sleep 1
str.clear #=> can't modify string; temporarily locked (RuntimeError)

I have attached two patches.
One fixes iofread(), another does iogetpartial() and rbiosysread().
May I commit these changes?

io_fread.diff Magnifier - for io_fread() (984 Bytes) Masaki Matsushita, 07/23/2013 06:06 PM

read_internal.diff Magnifier - for io_getpartial() and rb_io_sysread() (1.85 KB) Masaki Matsushita, 07/23/2013 06:06 PM

io_fread_callback.diff Magnifier - for io_fread() with rb_str_locktmp_ensure() (1.77 KB) Masaki Matsushita, 07/25/2013 01:59 PM

read_internal_callback.diff Magnifier - for io_getpartial() and rb_io_sysread() with rb_str_locktmp_ensure() (2.7 KB) Masaki Matsushita, 07/25/2013 01:59 PM

Associated revisions

Revision 42212
Added by glass 9 months ago

  • string.c: add internal API rbstrlocktmp_ensure().

  • io.c (iofread): use rbstrlocktmpensure().
    [Bug #8669]

  • test/ruby/test_io.rb: add a test for above.

Revision 42214
Added by glass 9 months ago

  • io.c (iogetpartial): use rbstrlocktmpensure().
    [Bug #8669]

  • io.c (rbiosysread): ditto.

  • test/ruby/test_io.rb: add tests for above.

History

#1 Updated by Eric Wong 9 months ago

"Glass_saga (Masaki Matsushita)" glass.saga@gmail.com wrote:

Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN

I can confirm the issue under 1.9.3-p448 and 2.0.0-p247

Following code make outbuf "temporarily" locked forever.
It is needed to ensure rbstrunlocktmp().

Nasty bug. I might've hit this soon in new project I'm working on(!)

I have attached two patches.
One fixes iofread(), another does iogetpartial() and rbiosysread().
May I commit these changes?

Untested, but looks correct. Thanks.

#2 Updated by Nobuyoshi Nakada 9 months ago

  • Status changed from Open to Assigned
  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 1.9.3: REQUIRED, 2.0.0: REQUIRED

Seems fine.

Furthermore, should we introduce a function to callback with locking
temporarily?

#3 Updated by Masaki Matsushita 9 months ago

Furthermore, should we introduce a function to callback with locking temporarily?

I made rbstrlocktmp_ensure() to callback with locktmp.

#4 Updated by Anonymous 9 months ago

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

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


  • string.c: add internal API rbstrlocktmp_ensure().

  • io.c (iofread): use rbstrlocktmpensure().
    [Bug #8669]

  • test/ruby/test_io.rb: add a test for above.

#5 Updated by Tomoyuki Chikanaga 9 months ago

  • Backport changed from 1.9.3: REQUIRED, 2.0.0: REQUIRED to 1.9.3: REQUIRED, 2.0.0: DONE

Backported to ruby20_0 at r42216.

#6 Updated by Usaku NAKAMURA 9 months ago

  • Backport changed from 1.9.3: REQUIRED, 2.0.0: DONE to 1.9.3: DONE, 2.0.0: DONE

Backported to ruby19_3 at r42327.

Also available in: Atom PDF