Bug #9040
closedReadline duplicate file descriptors but doesn't close them
Description
This depends on the max open files limit, happens quicker the lower the limit.
irb crashes just by holding down return. Uses two file descriptors per prompt.
input=': Too many open files - dup (Errno::EMFILE)
or if you don't want to hold down the key...
ulimit -n 100
ruby -r readline -e "100.times{ Readline.input = STDIN }"
A recent patch to readline to avoid a segv when the underlying FILE has been closed, changed the way that the input and output streams
are assigned.
When a stream is assigned, its file descriptor is extracted, dup'ed and passed to fdopen.
As the file descriptor is dup'ed the two file descriptors (in the FILE owned by the readline library and the one inside the ruby rb_io_t)
don't match.
Before assigning the previous value should be cleared. But this only happens when the ruby stream has been closed or when the two file descriptors are the same (never).
As we always dup the file descriptors, we own them, and should always close them.
Files
Updated by akr (Akira Tanaka) about 11 years ago
I think it's better to not trust the value of rl_instream and rl_outstream
because they can be modified by other libraries.
I updated my readline patch submitted to [Bug #8749].
Updated by normalperson (Eric Wong) about 11 years ago
"akr (Akira Tanaka)" akr@fsij.org wrote:
Issue #9040 has been updated by akr (Akira Tanaka).
File readline-release-gvl-3.patch added
Does poll work reliably with tty FD on non-Linux systems?
Perhaps better to use rb_thread_call_with_gvl and a
rb_wait_for_single_fd wrapper
Updated by akr (Akira Tanaka) about 11 years ago
normalperson (Eric Wong) wrote:
Does poll work reliably with tty FD on non-Linux systems?
Perhaps better to use rb_thread_call_with_gvl and a
rb_wait_for_single_fd wrapper
I see. I updated the patch.
Updated by normalperson (Eric Wong) about 11 years ago
"akr (Akira Tanaka)" akr@fsij.org wrote:
Issue #9040 has been updated by akr (Akira Tanaka).
File readline-release-gvl-4.patch added
Fewer ifdefs, so good :> (haven't tested)
Btw, on a separate note, it would be a good idea to check the return
value of fileno() in case another extension accidentally fclose()
Updated by akr (Akira Tanaka) about 11 years ago
normalperson (Eric Wong) wrote:
Btw, on a separate note, it would be a good idea to check the return
value of fileno() in case another extension accidentally fclose()
I think avoiding accidental fclose() is not the responsibility of readline_getc().
It is because readline_getc() cannot determine the FILE structure is closed or not.
So the only appropriate action for the situation is rb_bug()
because fileno() for a closed stream is undefined behavior
(possibly SEGV if memory for FILE structure is returned to OS).
I updated the patch.
Updated by normalperson (Eric Wong) about 11 years ago
"akr (Akira Tanaka)" akr@fsij.org wrote:
Issue #9040 has been updated by akr (Akira Tanaka).
File readline-release-gvl-5.patch added
normalperson (Eric Wong) wrote:
Btw, on a separate note, it would be a good idea to check the return
value of fileno() in case another extension accidentally fclose()I think avoiding accidental fclose() is not the responsibility of readline_getc().
It is because readline_getc() cannot determine the FILE structure is closed or not.
So the only appropriate action for the situation is rb_bug()
because fileno() for a closed stream is undefined behavior
(possibly SEGV if memory for FILE structure is returned to OS).
Agreed, thanks.
On a related note: should rb_fd_set/rb_fd_resize call rb_bug on
negative FD? Otherwise it could OOM/SEGV, I think
Updated by akr (Akira Tanaka) about 11 years ago
2013/10/24 Eric Wong normalperson@yhbt.net:
On a related note: should rb_fd_set/rb_fd_resize call rb_bug on
negative FD? Otherwise it could OOM/SEGV, I think
It's possible to fail fast.
I'm not sure wihch is suitable between rb_bug and rb_raise, though.
Negative FD can be determined safely.
Tanaka Akira
Updated by akr (Akira Tanaka) about 11 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
This issue was solved with changeset r43439.
Eamonn, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
-
ext/readline/readline.c: Include ruby/thread.h for
rb_thread_call_without_gvl2.
(readline_rl_instream, readline_rl_outstream): Record FILE
structures allocated by this extension.
(getc_body): New function extracted from readline_getc.
(getc_func): New function.
(readline_getc): Use rb_thread_call_without_gvl2 to invoke getc_func.
[ruby-dev:47033] [Bug #8749]
(clear_rl_instream, clear_rl_outstream): Close FILE structure
allocated by this extention reliably. [ruby-core:57951] [Bug #9040]
(readline_readline): Use clear_rl_instream and clear_rl_outstream.
(readline_s_set_input): Set readline_rl_instream.
(readline_s_set_output): Set readline_rl_outstream.
(Init_readline): Don't call readline_s_set_input because
readline_getc doesn't block other threads for any FILE structure now.[ruby-dev:47033] [Bug #8749] reported by Nobuhiro IMAI.
[ruby-core:57951] [Bug #9040] reporeted by Eamonn Webster.
Updated by wanabe (_ wanabe) almost 8 years ago
- Related to Bug #12950: irb: 'input-method.rb:151: [BUG] Segmentation fault' / 'malloc(): smallbin double linked list corrupted' added