You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ec...@apache.org on 2014/12/11 16:42:17 UTC

svn commit: r1644682 - in /commons/proper/vfs/trunk: core/src/main/java/org/apache/commons/vfs2/Resources.properties core/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java src/changes/changes.xml

Author: ecki
Date: Thu Dec 11 15:42:16 2014
New Revision: 1644682

URL: http://svn.apache.org/r1644682
Log:
[VFS-309] DefaultFileContent will remove thread data on close to avoid leaks

Modified:
    commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/Resources.properties
    commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java
    commons/proper/vfs/trunk/src/changes/changes.xml

Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/Resources.properties
URL: http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/Resources.properties?rev=1644682&r1=1644681&r2=1644682&view=diff
==============================================================================
--- commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/Resources.properties (original)
+++ commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/Resources.properties Thu Dec 11 15:42:16 2014
@@ -87,6 +87,7 @@ vfs.provider/get-certificates-writing.er
 vfs.provider/get-certificates.error=Could not retrieve the certificates of "{0}".
 vfs.provider/close-instr.error=Could not close the input stream for file "{0}".
 vfs.provider/close-outstr.error=Could not close the output stream for file "{0}".
+vfs.provider/close-rac.error=Could not close the random access content for file "{0}".
 vfs.provider/exists-attribute-no-exist.error=Could not check if attribute "{0}" of "{1}" exists because attributes are not supported.
 vfs.provider/get-attributes-no-exist.error=Could not get attributes for file "{0}" because it does not exist.
 vfs.provider/get-attributes.error=Could not get attributes "{0}".

Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java
URL: http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java?rev=1644682&r1=1644681&r2=1644682&view=diff
==============================================================================
--- commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java (original)
+++ commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java Thu Dec 11 15:42:16 2014
@@ -75,7 +75,7 @@ public final class DefaultFileContent im
         this.fileContentInfoFactory = fileContentInfoFactory;
     }
 
-    private FileContentThreadData getThreadData()
+    private FileContentThreadData getOrCreateThreadData()
     {
         FileContentThreadData data = this.threadData.get();
         if (data == null)
@@ -412,10 +412,9 @@ public final class DefaultFileContent im
 
         final InputStream wrappedInstr = new FileContentInputStream(fileObject, instr);
 
-        this.getThreadData().addInstr(wrappedInstr);
+        getOrCreateThreadData().addInstr(wrappedInstr);
         streamOpened();
 
-        // setState(STATE_OPENED);
         return wrappedInstr;
     }
 
@@ -440,10 +439,10 @@ public final class DefaultFileContent im
         final RandomAccessContent rastr = fileObject.getRandomAccessContent(mode);
 
         final FileRandomAccessContent rac = new FileRandomAccessContent(fileObject, rastr);
-        this.getThreadData().addRastr(rac);
+
+        getOrCreateThreadData().addRastr(rac);
         streamOpened();
 
-        // setState(STATE_OPENED);
         return rac;
     }
 
@@ -470,7 +469,8 @@ public final class DefaultFileContent im
         /*
         if (getThreadData().getState() != STATE_NONE)
         */
-        if (this.getThreadData().getOutstr() != null)
+        FileContentThreadData streams = getOrCreateThreadData();
+        if (streams.getOutstr() != null)
         {
             throw new FileSystemException("vfs.provider/write-in-use.error", fileObject);
         }
@@ -478,12 +478,12 @@ public final class DefaultFileContent im
         // Get the raw output stream
         final OutputStream outstr = fileObject.getOutputStream(bAppend);
 
-        // Create wrapper
-        this.getThreadData().setOutstr(new FileContentOutputStream(fileObject, outstr));
+        // Create and set wrapper
+        FileContentOutputStream wrapped = new FileContentOutputStream(fileObject, outstr);
+        streams.setOutstr(wrapped);
         streamOpened();
 
-        // setState(STATE_OPENED);
-        return this.getThreadData().getOutstr();
+        return wrapped;
     }
 
     /**
@@ -494,39 +494,65 @@ public final class DefaultFileContent im
     @Override
     public void close() throws FileSystemException
     {
+        FileSystemException caught = null;
         try
         {
+            final FileContentThreadData streams = getOrCreateThreadData();
+
             // Close the input stream
-            while (getThreadData().getInstrsSize() > 0)
+            while (streams.getInstrsSize() > 0)
             {
-                final FileContentInputStream instr = (FileContentInputStream) getThreadData().removeInstr(0);
-                instr.close();
+                final FileContentInputStream instr = (FileContentInputStream) streams.removeInstr(0);
+                try
+                {
+                    instr.close();
+                }
+                catch (final FileSystemException ex)
+                {
+                    caught = ex;
+
+                }
             }
 
             // Close the randomAccess stream
-            while (getThreadData().getRastrsSize() > 0)
+            while (streams.getRastrsSize() > 0)
             {
-                final RandomAccessContent ra = (RandomAccessContent) getThreadData().removeRastr(0);
+                final FileRandomAccessContent ra = (FileRandomAccessContent) streams.removeRastr(0);
                 try
                 {
                     ra.close();
                 }
-                catch (final IOException e)
+                catch (final FileSystemException ex)
                 {
-                    throw new FileSystemException(e);
+                    caught = ex;
                 }
             }
 
             // Close the output stream
-            if (this.getThreadData().getOutstr() != null)
+            final FileContentOutputStream outstr = streams.getOutstr();
+            if (outstr != null)
             {
-                this.getThreadData().closeOutstr();
+                streams.setOutstr(null);
+                try
+                {
+                    outstr.close();
+                }
+                catch (final FileSystemException ex)
+                {
+                    caught = ex;
+                }
             }
         }
         finally
         {
             threadData.remove();
         }
+
+        // throw last error (out >> rac >> input) after all closes have been tried
+        if (caught != null)
+        {
+            throw caught;
+        }
     }
 
     /**
@@ -534,14 +560,17 @@ public final class DefaultFileContent im
      */
     private void endInput(final FileContentInputStream instr)
     {
-        getThreadData().removeInstr(instr);
-        streamClosed();
-        /*
-        if (!getThreadData().hasStreams())
+        FileContentThreadData streams = threadData.get();
+        if (streams != null)
         {
-            setState(STATE_CLOSED);
+            streams.removeInstr(instr);
         }
-        */
+        if ((streams == null) || !streams.hasStreams())
+        {
+            // remove even when no value is set to remove key
+            threadData.remove();
+        }
+        streamClosed();
     }
 
     /**
@@ -549,9 +578,17 @@ public final class DefaultFileContent im
      */
     private void endRandomAccess(final RandomAccessContent rac)
     {
-        getThreadData().removeRastr(rac);
+        FileContentThreadData streams = threadData.get();
+        if (streams != null)
+        {
+            streams.removeRastr(rac);
+        }
+        if ((streams == null) || !streams.hasStreams())
+        {
+            // remove even when no value is set to remove key
+            threadData.remove();
+        }
         streamClosed();
-        // setState(STATE_CLOSED);
     }
 
     /**
@@ -559,21 +596,20 @@ public final class DefaultFileContent im
      */
     private void endOutput() throws Exception
     {
+        FileContentThreadData streams = threadData.get();
+        if (streams != null)
+        {
+            streams.setOutstr(null);
+        }
+        if ((streams == null) || !streams.hasStreams())
+        {
+            // remove even when no value is set to remove key
+            threadData.remove();
+        }
         streamClosed();
-
-        this.getThreadData().setOutstr(null);
-        // setState(STATE_CLOSED);
-
         fileObject.endOutput();
     }
 
-    /*
-    private void setState(int state)
-    {
-        getThreadData().setState(state);
-    }
-    */
-
     /**
      * Check if a input and/or output stream is open.
      * <p>
@@ -584,8 +620,15 @@ public final class DefaultFileContent im
     @Override
     public boolean isOpen()
     {
-        // return getThreadData().getState() == STATE_OPENED;
-        return getThreadData().hasStreams();
+        FileContentThreadData streams = threadData.get();
+        if (streams != null && streams.hasStreams())
+        {
+            return true;
+        } else {
+            // threadData.get() created empty entry
+            threadData.remove(); 
+            return false;
+        }
     }
 
     /**
@@ -655,18 +698,13 @@ public final class DefaultFileContent im
      */
     private final class FileRandomAccessContent extends MonitorRandomAccessContent
     {
-        // avoid gc
-        @SuppressWarnings("unused")
+        // also avoids gc
         private final FileObject file;
 
-        @SuppressWarnings("unused")
-        private final RandomAccessContent content;
-
         FileRandomAccessContent(final FileObject file, final RandomAccessContent content)
         {
             super(content);
             this.file = file;
-            this.content = content;
         }
 
         /**
@@ -684,6 +722,19 @@ public final class DefaultFileContent im
                 endRandomAccess(this);
             }
         }
+
+        @Override
+        public void close() throws FileSystemException
+        {
+            try
+            {
+                super.close();
+            }
+            catch (final IOException e)
+            {
+                throw new FileSystemException("vfs.provider/close-rac.error", file, e);
+            }
+        }
     }
 
     /**

Modified: commons/proper/vfs/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/vfs/trunk/src/changes/changes.xml?rev=1644682&r1=1644681&r2=1644682&view=diff
==============================================================================
--- commons/proper/vfs/trunk/src/changes/changes.xml (original)
+++ commons/proper/vfs/trunk/src/changes/changes.xml Thu Dec 11 15:42:16 2014
@@ -26,6 +26,9 @@
 <!--       <action issue="VFS-443" dev="ggregory" type="update" due-to="nickallen"> -->
 <!--        [Local] Need an easy way to convert from a FileObject to a File. -->
 <!--       </action> -->
+      <action issue="VFS-309" dev="ecki" type="fix">
+       DefaultFileContent will remove thread data whenever possible to avoid leaks.
+      </action>
       <action issue="VFS-487" dev="ecki" type="fix" due-to="Dave Marion">
        DefaultFileMonitor detects recreated files.
       </action>