Project

General

Profile

Actions

Feature #16962

closed

Make IO.for_fd autoclose option default to false

Added by ioquatix (Samuel Williams) about 2 years ago. Updated about 1 year ago.

Status:
Feedback
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:98825]

Description

I discussed this with @Eregon (Benoit Daloze) and I think the goal here is to try and figure out a way these interfaces can be a bit less confusing.

1. I don't understand this behaviour:

STDOUT.close
STDOUT.puts "Hello World"
# => closed stream

vs

IO.for_fd(STDOUT.fileno, autoclose: true).close
STDOUT.puts "Hello World"
# => Hello World

2. IO.for_fd(..., autoclose: true/false)

The documentation for autoclose is:

If the value is false, the fd will be kept open after this IO instance gets finalized.

But it also seems to affect #close - i.e. calling close does not close underlying file descriptor.

Should we fix the documentation or is the implementation wrong? Maybe the name autoclose: is very confusing. My initial interpretation was it was just 'automatically close this I/O when it is garbage collected'.

3. IO.for_fd(..., autoclose: false) default

In most cases, it seems like autoclose: false would make more sense as the default, since the file descriptor must come from some other place.

Actions #1

Updated by ioquatix (Samuel Williams) about 2 years ago

  • Description updated (diff)
Actions #2

Updated by ioquatix (Samuel Williams) about 2 years ago

  • Description updated (diff)

Updated by jeremyevans0 (Jeremy Evans) about 1 year ago

  • Backport deleted (2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN)
  • Subject changed from IO.close behaviour to Make IO.for_fd autoclose option default to false
  • Tracker changed from Bug to Feature

I looked into this and I think it's because stdin, stdout, and stderr are special cased:

    if (IS_PREP_STDIO(fptr) || fd <= 2) {
        /* need to keep FILE objects of stdin, stdout and stderr */
    }

This Ruby program shows the expected behavior:

out = STDOUT.dup
IO.for_fd(out.fileno, autoclose: true).close
out.puts "Hello World"

output:

-:3:in `write': Bad file descriptor @ io_writev - <STDOUT> (Errno::EBADF)
        from -:3:in `puts'
        from -:3:in `<main>'

I'm not sure it is worth documenting that autoclose does not affect stdin, stdout, or stderr.

I agree that autoclose: false is probably a better default. However, that's a feature request and not a bug fix. Updating subject and switching to feature.

Updated by ioquatix (Samuel Williams) about 1 year ago

Thanks for looking into this.

Not being able to close some file descriptors when using IO.for_fd is almost certainly a bug.

/* need to keep FILE objects of stdin, stdout and stderr */

Why?

Updated by jeremyevans0 (Jeremy Evans) about 1 year ago

ioquatix (Samuel Williams) wrote in #note-4:

Thanks for looking into this.

Not being able to close some file descriptors when using IO.for_fd is almost certainly a bug.

/* need to keep FILE objects of stdin, stdout and stderr */

Why?

My git blame magic ball shows that Ruby has always done this. It was temporarily changed in 6f1edacc010c48c5762598fed74ff4f1e62d1b20 and then quickly changed back in [ruby-dev:38648] * io.c (fptr_finalize): skip close..." href="/projects/ruby-master/repository/git/revisions/d3b852345a646f1058b40033c9f02e3d14c45ab6">d3b852345a646f1058b40033c9f02e3d14c45ab6 (both by @matz (Yukihiro Matsumoto)).

Updated by akr (Akira Tanaka) about 1 year ago

We don't close 0 (stdin), 1 (stdout), 2 (stderr) because
they can be used from various library.

If we close 2, next open() return 2.
Assume that we open a database file.
If open() returns 2 for the database,
the database would be corrupted when ruby raises an error that error messages are printed to stderr.

I feel that IO.for_fd with autoclose=true is appropriate for
applications which takes a file descriptor as a command line option.
(valgrind --log-fd=FD, etc.)
In that case, IO object created by IO.for_fd should closed when the IO object is garbage collected.

I agree that autoclose=false is appropriate for testing but
I guess real applications need autoclose=true more.

Actions #7

Updated by akr (Akira Tanaka) about 1 year ago

  • Status changed from Open to Feedback
Actions

Also available in: Atom PDF