Bug #8669

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

Added by Masaki Matsushita about 2 years ago. Updated about 2 years ago.

[ruby-core:56121]
Status:Closed
Priority:Normal
Assignee:Masaki Matsushita
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 rb_str_unlocktmp().

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 io_fread(), another does io_getpartial() and rb_io_sysread().
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 about 2 years ago

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

  • io.c (io_fread): use rb_str_locktmp_ensure().
    [Bug #8669]

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

Revision 42212
Added by glass about 2 years ago

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

  • io.c (io_fread): use rb_str_locktmp_ensure().
    [Bug #8669]

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

Revision 42214
Added by glass about 2 years ago

  • io.c (io_getpartial): use rb_str_locktmp_ensure().
    [Bug #8669]

  • io.c (rb_io_sysread): ditto.

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

Revision 42214
Added by glass about 2 years ago

  • io.c (io_getpartial): use rb_str_locktmp_ensure().
    [Bug #8669]

  • io.c (rb_io_sysread): ditto.

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

History

#1 Updated by Eric Wong about 2 years 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 rb_str_unlocktmp().

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

I have attached two patches.
One fixes io_fread(), another does io_getpartial() and rb_io_sysread().
May I commit these changes?

Untested, but looks correct. Thanks.

#2 Updated by Nobuyoshi Nakada about 2 years ago

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

Seems fine.

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

#3 Updated by Masaki Matsushita about 2 years ago

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

I made rb_str_locktmp_ensure() to callback with locktmp.

#4 Updated by Anonymous about 2 years 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 rb_str_locktmp_ensure().

  • io.c (io_fread): use rb_str_locktmp_ensure().
    [Bug #8669]

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

#5 Updated by Tomoyuki Chikanaga about 2 years ago

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

Backported to ruby_2_0_0 at r42216.

#6 Updated by Usaku NAKAMURA about 2 years ago

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

Backported to ruby_1_9_3 at r42327.

Also available in: Atom PDF