Project

General

Profile

Feature #7148

Improved Tempfile w/o DelegateClass

Added by Masaki Matsushita about 4 years ago. Updated over 1 year ago.

Status:
Assigned
Priority:
Normal
[ruby-core:47930]

Description

I propose improved Tempfile without DelegateClass().
Present Tempfile has following problems.

  1. confusing inspect

    t = Tempfile.new("foo") #=> #<File:/tmp/foo20121012-6762-12w11to>
    t.is_a? File #=> false
    
  2. #dup doesn't duplicate IO

    t = Tempfile.new("foo")
    t.dup.close
    t.read #=> IOError: closed stream
    
  3. finalizer performs unlink even when it has been duplicated

    t = Tempfile.new("foo")
    path = t.path #=> "/tmp/foo20121012-7533-1q537gq"
    File.exist? path #=> true
    tt = t.dup
    t = nil
    GC.start
    File.exist? path #=> false
    

I think these problems caused by using DelegateClass().
Therefore, I made a patch to resolve the problems.
The patched Tempfile class is a subclass of File.

patch.diff View (3.52 KB) Masaki Matsushita, 10/12/2012 02:04 PM

History

#1 [ruby-core:47932] Updated by Anonymous about 4 years ago

Hi,

In message "Re: [ruby-trunk - Feature #7148][Open] Improved Tempfile w/o DelegateClass"
on Fri, 12 Oct 2012 14:04:08 +0900, "Glass_saga (Masaki Matsushita)" glass.saga@gmail.com writes:

I propose improved Tempfile without DelegateClass().
Present Tempfile has following problems.

  1. confusing inspect
  2. #dup doesn't duplicate IO
  3. finalizer performs unlink even when it has been duplicated

I have no objection about (1), but what we expect when we call
Tempfile#dup might differ, for example, I'd expect it to copy the
underlying temporary file. So making Tempfile a subclass of File
may not be the ideal solution.

                        matz.

#2 [ruby-core:48015] Updated by Masaki Matsushita about 4 years ago

Hello,

Yukihiro Matsumoto wrote:

I'd expect it to copy the underlying temporary file.

Is the behavior of #dup you expect like the following?

def dup
  dupe = self.class.new(@basename)
  IO.copy_stream(self, dupe, 0)
  dupe
end

I think the reason why Tempfile uses DelegateClass is that to implement Tempfile#open without it used to be difficult.
To implement it as subclass of File, self must be reopened with full configuration, mode and opts.
IO#reopen used not to accept them, but now it accepts after r37071.

#3 [ruby-core:48054] Updated by Charles Nutter about 4 years ago

JRuby has been running with Tempfile < File for a couple years now, and have received only minor bug reports about it. It works very well, and has no delegation cost.

#4 [ruby-core:49408] Updated by Masaki Matsushita about 4 years ago

Are there some reasons not to make Tempfile a subclass of File?
I think it's a better solution, even if it's not an ideal solution.

#5 [ruby-core:49955] Updated by Yusuke Endoh about 4 years ago

  • Target version set to next minor
  • Status changed from Open to Assigned
  • Assignee set to Yukihiro Matsumoto
  • Priority changed from Normal to 3

#6 [ruby-core:69385] Updated by Eric Wong over 1 year ago

"Glass_saga (Masaki Matsushita)" glass.saga@gmail.com wrote:

Feature #7148: Improved Tempfile w/o DelegateClass
https://bugs.ruby-lang.org/issues/7148

I would still like this for 2.3.0, just hit a snag with IO.copy_stream
using Tempfile :x

Also, Charles hit a similar problem not long ago, too:
[Bug #11015]

#7 [ruby-core:69406] Updated by Masaki Matsushita over 1 year ago

I think the problem you faced was resolved at r50118, wasn't it?

#8 [ruby-core:69407] Updated by Eric Wong over 1 year ago

Sadly, r50118 actually caused my problem because I was using src_offset
for IO.copy_stream

#9 [ruby-core:69418] Updated by Charles Nutter over 1 year ago

It doesn't sound like anyone opposes this idea, so are we just missing implementation?

#10 [ruby-core:69528] Updated by Nobuyoshi Nakada over 1 year ago

  • Description updated (diff)

#11 [ruby-core:69529] Updated by Yukihiro Matsumoto over 1 year ago

It's OK for me to implement Tempfile without using DelegateClass. If JRuby does similar thing already, it might be a good idea to share implementation. Besides that, we might need to think about killing/redefining some unnecessary/invalid methods of File class, e.g. reopen. I am afraid dup is one of them.

Matz.

#12 [ruby-core:69589] Updated by Masaki Matsushita over 1 year ago

  • Assignee changed from Yukihiro Matsumoto to Masaki Matsushita

Also available in: Atom PDF