Project

General

Profile

Actions

Feature #10718

closed

IO#close should not raise IOError on closed IO objects.

Added by akr (Akira Tanaka) about 9 years ago. Updated almost 9 years ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:67444]

Description

I'd like to change IO#close.
It should not raise IOError on closed IO objects.

We sometimes invoke IO#close only when the IO object is not closed as:

f.close if !f.closed?

If this issue is accepted, we can write it simply as follows.

f.close

Simple grep finds many examples.
Following examples are just a little excerpt.

lib/webrick/server.rb:          sock.close unless sock.closed?
lib/pstore.rb:          file.close if !file.closed?
lib/mkmf.rb:          @log.close if @log and not @log.closed?
lib/cgi/session.rb:          f.close if f and !f.closed?
lib/open-uri.rb:          io.close if !io.closed?
lib/net/pop.rb:        s.close if s and not s.closed?
lib/net/http.rb:          @socket.close if @socket and not @socket.closed?
lib/net/smtp.rb:        s.close if s and not s.closed?
lib/shell/process-controller.rb:                io.close unless io.closed?
test/ruby/test_io.rb:    w.close unless !w || w.closed?
...

I think there is no problem with the behavior which IO#close doesn't raise an exception on closed IO object.
Because the closed state fulfils the postcondition of the method.
The change means relaxing the precondition which should be harmless.

Moreover raising IOError can smash other exceptions if it is called in ensure clause.
It makes debugging difficult and the proposed behavior ease it.

It also useful to close IO object asynchronously.
Asynchronous close can be used for graceful shutdown.
This issue can eliminate "rescue IOError".
See lib/webrick/server.rb and lib/drb/drb.rb for example.

Note that "double close" is a bad idea in C.
But it is not applicable to Ruby.
A FILE structure is freed on fclose().
But Ruby's IO object is not freed until GC.
So method invocation on closed object doesn't cause invalid memory access.
A file descriptor (FD) can be reused any time because a signal handler or another thread may allocate a FD.
But Ruby's IO object is not reused after close.
So it is impossible to close an IO object unintentionally.


Files

io-close.patch (1.97 KB) io-close.patch akr (Akira Tanaka), 01/09/2015 01:44 AM

Related issues 1 (0 open1 closed)

Related to Ruby master - Bug #13405: IO#close raises "stream closed"ClosedActions

Updated by normalperson (Eric Wong) about 9 years ago

Thank you for proposing this. I think this will simplify working with
Ruby IO and make me happier since I work with a lot of IO-related code.
Outside of test cases, I don't forsee compatibility problems either.

Updated by zzak (zzak _) about 9 years ago

I think the API is improved but I'm not sure we need to find & replace
every occurrence in the stdlib

Updated by akr (Akira Tanaka) about 9 years ago

I don't expect immediate stdlib update.

Updated by akr (Akira Tanaka) about 9 years ago

  • Description updated (diff)

Updated by akr (Akira Tanaka) about 9 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

Applied in changeset r49260.


Updated by akr (Akira Tanaka) about 9 years ago

I met matz at yesterday (2015-01-15) and he accept this issue.

Updated by normalperson (Eric Wong) almost 9 years ago

Eric Wong wrote:

Thank you for proposing this. I think this will simplify working with
Ruby IO and make me happier since I work with a lot of IO-related code.
Outside of test cases, I don't forsee compatibility problems either.

Ugh, I take that back...

This can hide existing bugs. For example, I track open connection
counts (persistent HTTP connections are expired in a GC-like manner)
in the multi-threaded yahns server[1]. Something like:

@mtx.synchronize do
  @count -= 1
  io.close
end

If I get IOError as in 2.2, I'll there is a major bug (probably race
condition) fix the code. But now, I may need to add extra checks for
io.closed? before every io.close:

@mtx.synchronize do
  @count -= 1
  raise "MAJOR BUG!" if io.closed?
  io.close
end

Anyways, I'm not sure if many people complained about IOError on
IO#close over the past years; so perhaps reverting to 2.2 behavior is
safer.

[1] http://yahns.yhbt.net/README || git clone git://yhbt.net/yahns

Updated by akr (Akira Tanaka) almost 9 years ago

I feel there are much more useful situations than disappointment situations related to this change.

Actions #9

Updated by shyouhei (Shyouhei Urabe) almost 7 years ago

  • Related to Bug #13405: IO#close raises "stream closed" added
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0