Not a member of Pastebin yet?
Sign Up,
it unlocks many cool features!
- commit 7050bc18a592b779d8fb102ef7ce583902763e3a
- Author: Vladimir Sizikov <vsizikov@gmail.com>
- Date: Sat Mar 6 13:04:33 2010 +0100
- JRUBY-4623: Tempfile does not clean up on GC run
- Added unit test as well, but didn't wire it to the index,
- since these GC-related issues are not 100% reliably reproducible.
- diff --git a/src/org/jruby/RubyTempfile.java b/src/org/jruby/RubyTempfile.java
- index b77b9f4..874b68d 100644
- --- a/src/org/jruby/RubyTempfile.java
- +++ b/src/org/jruby/RubyTempfile.java
- @@ -30,6 +30,9 @@ package org.jruby;
- import java.io.File;
- import java.io.IOException;
- +import java.util.concurrent.ConcurrentHashMap;
- +import java.util.concurrent.ConcurrentMap;
- +
- import org.jruby.anno.JRubyClass;
- import org.jruby.anno.JRubyMethod;
- import org.jruby.platform.Platform;
- @@ -38,14 +41,22 @@ import org.jruby.runtime.ObjectAllocator;
- import org.jruby.runtime.ThreadContext;
- import org.jruby.runtime.Visibility;
- import org.jruby.runtime.builtin.IRubyObject;
- +import org.jruby.util.ReferenceReaper;
- import org.jruby.util.io.InvalidValueException;
- import org.jruby.util.io.ModeFlags;
- +import org.jruby.util.io.OpenFile;
- /**
- * An implementation of tempfile.rb in Java.
- */
- @JRubyClass(name="Tempfile", parent="File")
- public class RubyTempfile extends RubyFile {
- +
- + /** Keep strong references to the Reaper until cleanup */
- + private static final ConcurrentMap<Reaper, Boolean> referenceSet
- + = new ConcurrentHashMap<Reaper, Boolean>();
- + private transient volatile Reaper reaper;
- +
- private static ObjectAllocator TEMPFILE_ALLOCATOR = new ObjectAllocator() {
- public IRubyObject allocate(Ruby runtime, RubyClass klass) {
- RubyFile instance = new RubyTempfile(runtime, klass);
- @@ -116,6 +127,7 @@ public class RubyTempfile extends RubyFile {
- path = tmp.getPath();
- tmpFile.deleteOnExit();
- initializeOpen();
- + referenceSet.put(reaper = new Reaper(this, runtime, tmpFile, openFile), Boolean.TRUE);
- return this;
- }
- } catch (IOException e) {
- @@ -203,6 +215,8 @@ public class RubyTempfile extends RubyFile {
- @JRubyMethod(name = "close!", frame = true, visibility = Visibility.PUBLIC)
- public IRubyObject close_bang(ThreadContext context) {
- + referenceSet.remove(reaper);
- + reaper.released = true;
- _close(context);
- tmpFile.delete();
- return context.getRuntime().getNil();
- @@ -210,7 +224,10 @@ public class RubyTempfile extends RubyFile {
- @JRubyMethod(name = {"unlink", "delete"}, frame = true)
- public IRubyObject unlink(ThreadContext context) {
- - if (tmpFile.exists()) tmpFile.delete();
- + if (!tmpFile.exists() || tmpFile.delete()) {
- + referenceSet.remove(reaper);
- + reaper.released = true;
- + }
- return context.getRuntime().getNil();
- }
- @@ -241,4 +258,44 @@ public class RubyTempfile extends RubyFile {
- return tempfile;
- }
- +
- + private static final class Reaper extends ReferenceReaper.Phantom<RubyTempfile> implements Runnable {
- + private volatile boolean released = false;
- + private final Ruby runtime;
- + private final File tmpFile;
- + private final OpenFile openFile;
- +
- + Reaper(RubyTempfile file, Ruby runtime, File tmpFile, OpenFile openFile) {
- + super(file);
- + this.runtime = runtime;
- + this.tmpFile = tmpFile;
- + this.openFile = openFile;
- + }
- +
- + public final void run() {
- + referenceSet.remove(this);
- + release();
- + clear();
- + }
- +
- + final void release() {
- + if (!released) {
- + released = true;
- + if (openFile != null) {
- + openFile.cleanup(runtime, false);
- + }
- + if (tmpFile.exists()) {
- + boolean deleted = tmpFile.delete();
- + if (runtime.getDebug().isTrue()) {
- + String msg = "removing " + tmpFile.getPath() + " ... ";
- + if (deleted) {
- + runtime.getErr().println(msg + "done");
- + } else {
- + runtime.getErr().println(msg + "can't delete");
- + }
- + }
- + }
- + }
- + }
- + }
- }
- diff --git a/test/test_tempfile_cleanup.rb b/test/test_tempfile_cleanup.rb
- new file mode 100644
- index 0000000..f95fff0
- --- /dev/null
- +++ b/test/test_tempfile_cleanup.rb
- @@ -0,0 +1,35 @@
- +require 'tempfile'
- +require 'java' if defined?(JRUBY_VERSION)
- +require 'test/unit'
- +require 'fileutils'
- +
- +class TestTempfilesCleanUp < Test::Unit::TestCase
- +
- + def setup
- + @tmpdir = "tmp_#{$$}"
- + FileUtils.rm_f @tmpdir rescue nil
- + Dir.mkdir @tmpdir rescue nil
- + end
- +
- + def teardown
- + FileUtils.rm_f @tmpdir
- + end
- +
- + def test_cleanup
- + 10.times { Tempfile.open('blah', @tmpdir) }
- +
- + # check that files were created
- + assert Dir["#{@tmpdir}/*"].size > 0
- +
- + 100.times do
- + if defined?(JRUBY_VERSION)
- + java.lang.System.gc
- + else
- + GC.start
- + end
- + end
- +
- + # test that the files are gone
- + assert_equal 0, Dir["#{@tmpdir}/*"].size, 'Files were not cleaned up'
- + end
- +end
Add Comment
Please, Sign In to add comment