JRuby

Tempfile#{unlink,delete} should warn or actualy do something

Details

  • Type: Improvement Improvement
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: JRuby 1.7.0.pre1
  • Fix Version/s: JRuby 1.7.0.pre2
  • Component/s: Intro, Standard Library
  • Labels:
    None
  • Number of attachments :
    1

Description

Due to https://github.com/jruby/jruby/commit/43bd20f28fa1854, Tempfile#

{unlink,delete}

now do nothing in JRuby.

The comment was very helpful in my search, thanks Charles!

I think the situation could be improved by warning when someone is using unlink or delete.

This behavior being not compatible with MRI, and calling unlink/delete after close(false) actually not deleting the file is quite surprising.

Maybe unlink/delete could actually delete the file if it "isClosed()" before (with #close(false)) ?

This is not a major problem since the proper way to unlink the tempfile is using #close(true). But I expect `tmp.close; tmp.delete` to be similar to tmp.close(true).

Issue Links

relates to

Bug - A problem which impairs or prevents the functions of the product. JRUBY-6477 Tempfile#stat raises java.lang.NullPointerException when unlinked

  • Major - Major loss of function.
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Benoit Daloze added a comment -

This example of `tmp.close; tmp.delete` is actually the first "Good practice" in the documentation of Tempfile:

# Therefore, one should always call #unlink or close in an ensure block, like
# this:
#
#   file = Tempfile.new('foo')
#   begin
#      ...do something with file...
#   ensure
#      file.close
#      file.unlink   # deletes the temp file
#   end
Show
Benoit Daloze added a comment - This example of `tmp.close; tmp.delete` is actually the first "Good practice" in the documentation of Tempfile: # Therefore, one should always call #unlink or close in an ensure block, like # this: # # file = Tempfile.new('foo') # begin # ...do something with file... # ensure # file.close # file.unlink # deletes the temp file # end
Hiro Asari made changes -
Field Original Value New Value
Description Due to 43bd20f28fa185430f7bf06f529db0221f9e79c3, Tempfile#{unlink,delete} now do nothing in JRuby.

The comment was very helpful in my search, thanks Charles!

I think the situation could be improved by warning when someone is using unlink or delete.

This behavior being not compatible with MRI, and calling unlink/delete after close(false) actually not deleting the file is quite surprising.

Maybe unlink/delete could actually delete the file if it "isClosed()" before (with #close(false)) ?

This is not a major problem since the proper way to unlink the tempfile is using #close(true). But I expect `tmp.close; tmp.delete` to be similar to tmp.close(true). 		Due to https://github.com/jruby/jruby/commit/43bd20f28fa1854, Tempfile#{unlink,delete} now do nothing in JRuby.

The comment was very helpful in my search, thanks Charles!

I think the situation could be improved by warning when someone is using unlink or delete.

This behavior being not compatible with MRI, and calling unlink/delete after close(false) actually not deleting the file is quite surprising.

Maybe unlink/delete could actually delete the file if it "isClosed()" before (with #close(false)) ?

This is not a major problem since the proper way to unlink the tempfile is using #close(true). But I expect `tmp.close; tmp.delete` to be similar to tmp.close(true).
Hiro Asari made changes -
Link This issue relates to JRUBY-6477 [ JRUBY-6477 ]
Hiro Asari made changes -
Component/s Intro [ 14308 ]
Hide
Permalink
Benoit Daloze added a comment -

Attached proposed fix: actual delete when closed.

Show
Benoit Daloze added a comment - Attached proposed fix: actual delete when closed.
Benoit Daloze made changes -
Attachment 0001-Fix-Tempfile-unlink-actually-delete-the-file-if-it-i.patch [ 60115 ]
Hide
Permalink
Charles Oliver Nutter added a comment -

I like the fix and will also add a warning for unlink/delete when isClosed == false.

Show
Charles Oliver Nutter added a comment - I like the fix and will also add a warning for unlink/delete when isClosed == false.
Hide
Permalink
Charles Oliver Nutter added a comment - 
commit 48f61bceaa04ebaa0b8587e765e50851d27276c3
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Mon Jul 2 16:16:20 2012 -0500

    Add a warning for unlink/delete against open Tempfile. JRUBY-6688

:100644 100644 591eb2a... 859162d... M	src/org/jruby/ext/tempfile/Tempfile.java

commit 5c75386f4a1ce6af845a06ca4836b06240fb8b97
Author: Benoit Daloze <eregontp@gmail.com>
Date:   Thu May 31 11:02:57 2012 +0200

    Fix Tempfile#unlink: actually delete the file if it is closed before
    
    Signed-off-by: Charles Oliver Nutter <headius@headius.com>

:100644 000000 13c2876... 0000000... D	spec/tags/1.9/ruby/library/tempfile/delete_tags.txt
:100644 000000 d824287... 0000000... D	spec/tags/1.9/ruby/library/tempfile/unlink_tags.txt
:100644 100644 d0efd33... 591eb2a... M	src/org/jruby/ext/tempfile/Tempfile.java
Show
Charles Oliver Nutter added a comment - commit 48f61bceaa04ebaa0b8587e765e50851d27276c3 Author: Charles Oliver Nutter <headius@headius.com> Date: Mon Jul 2 16:16:20 2012 -0500 Add a warning for unlink/delete against open Tempfile. JRUBY-6688 :100644 100644 591eb2a... 859162d... M src/org/jruby/ext/tempfile/Tempfile.java commit 5c75386f4a1ce6af845a06ca4836b06240fb8b97 Author: Benoit Daloze <eregontp@gmail.com> Date: Thu May 31 11:02:57 2012 +0200 Fix Tempfile#unlink: actually delete the file if it is closed before Signed-off-by: Charles Oliver Nutter <headius@headius.com> :100644 000000 13c2876... 0000000... D spec/tags/1.9/ruby/library/tempfile/delete_tags.txt :100644 000000 d824287... 0000000... D spec/tags/1.9/ruby/library/tempfile/unlink_tags.txt :100644 100644 d0efd33... 591eb2a... M src/org/jruby/ext/tempfile/Tempfile.java
Charles Oliver Nutter made changes -
Status Open [ 1 ] Resolved [ 5 ]
Assignee Thomas E Enebo [ enebo ] Charles Oliver Nutter [ headius ]
Fix Version/s JRuby 1.7.0.pre2 [ 18527 ]
Resolution Fixed [ 1 ]

People

  • Assignee:
    Charles Oliver Nutter
    Reporter:
    Benoit Daloze
Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: