Project

General

Profile

Actions

Feature #16962

open

Make IO.for_fd autoclose option default to false

Added by ioquatix (Samuel Williams) over 1 year ago. Updated 3 months 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) over 1 year ago

  • Description updated (diff)
Actions #2

Updated by ioquatix (Samuel Williams) over 1 year ago

  • Description updated (diff)

Updated by jeremyevans0 (Jeremy Evans) 5 months 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) 5 months 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) 5 months 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 d3b852345a646f1058b40033c9f02e3d14c45ab6 (both by matz (Yukihiro Matsumoto)).

Updated by akr (Akira Tanaka) 3 months 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) 3 months ago

  • Status changed from Open to Feedback
Actions

Also available in: Atom PDF