Project

General

Profile

Actions

Bug #9040

closed

Readline duplicate file descriptors but doesn't close them

Added by eweb (Eamonn Webster) over 10 years ago. Updated over 10 years ago.

Status:
Closed
Assignee:
-
Target version:
ruby -v:
ruby 2.1.0dev (2013-09-22 trunk 43011) [x86_64-darwin12.5.0]
[ruby-core:57951]

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

readline_fix.patch (2.51 KB) readline_fix.patch eweb (Eamonn Webster), 10/21/2013 09:36 PM
readline-release-gvl-3.patch (8.06 KB) readline-release-gvl-3.patch akr (Akira Tanaka), 10/21/2013 11:51 PM
readline-release-gvl-4.patch (7.44 KB) readline-release-gvl-4.patch akr (Akira Tanaka), 10/22/2013 07:47 PM
readline-release-gvl-5.patch (7.99 KB) readline-release-gvl-5.patch akr (Akira Tanaka), 10/23/2013 06:17 PM

Related issues 1 (0 open1 closed)

Related to Ruby master - Bug #12950: irb: 'input-method.rb:151: [BUG] Segmentation fault' / 'malloc(): smallbin double linked list corrupted'Third Party's IssueActions

Updated by akr (Akira Tanaka) over 10 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) over 10 years ago

"akr (Akira Tanaka)" 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

Actions #3

Updated by akr (Akira Tanaka) over 10 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) over 10 years ago

"akr (Akira Tanaka)" 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()

Actions #5

Updated by akr (Akira Tanaka) over 10 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) over 10 years ago

"akr (Akira Tanaka)" 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) over 10 years ago

2013/10/24 Eric Wong :

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

Actions #8

Updated by akr (Akira Tanaka) over 10 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.

Actions #9

Updated by wanabe (_ wanabe) over 7 years ago

  • Related to Bug #12950: irb: 'input-method.rb:151: [BUG] Segmentation fault' / 'malloc(): smallbin double linked list corrupted' added
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0