You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ace.apache.org by ma...@apache.org on 2012/03/28 07:35:42 UTC

svn commit: r1306176 - in /ace/trunk: ace-configurator/ ace-configurator/src/test/java/org/apache/ace/configurator/ ace-deployment-verifier/ ace-obr-metadata/ ace-obr-metadata/src/main/java/org/apache/ace/obr/metadata/bindex/ ace-obr-storage/ ace-obr-s...

Author: marrs
Date: Wed Mar 28 05:35:42 2012
New Revision: 1306176

URL: http://svn.apache.org/viewvc?rev=1306176&view=rev
Log:
ACE-155 applied the patch

Modified:
    ace/trunk/ace-configurator/pom.xml
    ace/trunk/ace-configurator/src/test/java/org/apache/ace/configurator/ConfiguratorTest.java
    ace/trunk/ace-deployment-verifier/pom.xml
    ace/trunk/ace-obr-metadata/pom.xml
    ace/trunk/ace-obr-metadata/src/main/java/org/apache/ace/obr/metadata/bindex/BIndexMetadataGenerator.java
    ace/trunk/ace-obr-storage/pom.xml
    ace/trunk/ace-obr-storage/src/main/java/org/apache/ace/obr/storage/file/BundleFileStore.java
    ace/trunk/ace-repository-impl/src/main/java/org/apache/ace/repository/impl/RepositoryImpl.java
    ace/trunk/pom/pom.xml

Modified: ace/trunk/ace-configurator/pom.xml
URL: http://svn.apache.org/viewvc/ace/trunk/ace-configurator/pom.xml?rev=1306176&r1=1306175&r2=1306176&view=diff
==============================================================================
--- ace/trunk/ace-configurator/pom.xml (original)
+++ ace/trunk/ace-configurator/pom.xml Wed Mar 28 05:35:42 2012
@@ -70,6 +70,11 @@
             <groupId>org.apache.ace</groupId>
             <artifactId>org.apache.ace.util</artifactId>
         </dependency>
+        <dependency>
+            <groupId>commons-io</groupId>
+            <artifactId>commons-io</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
 </project>

Modified: ace/trunk/ace-configurator/src/test/java/org/apache/ace/configurator/ConfiguratorTest.java
URL: http://svn.apache.org/viewvc/ace/trunk/ace-configurator/src/test/java/org/apache/ace/configurator/ConfiguratorTest.java?rev=1306176&r1=1306175&r2=1306176&view=diff
==============================================================================
--- ace/trunk/ace-configurator/src/test/java/org/apache/ace/configurator/ConfiguratorTest.java (original)
+++ ace/trunk/ace-configurator/src/test/java/org/apache/ace/configurator/ConfiguratorTest.java Wed Mar 28 05:35:42 2012
@@ -105,7 +105,7 @@ public class ConfiguratorTest {
                 if (dest.exists()) {
                     dest.delete();
                 }
-                outFile.renameTo(dest);
+                renameFile(outFile, dest);
             }
             else {
                 File file = new File(m_configDir, factoryPid);
@@ -114,7 +114,7 @@ public class ConfiguratorTest {
                 if (dest.exists()) {
                     dest.delete();
                 }
-                outFile.renameTo(dest);
+                renameFile(outFile, dest);
             }
         }
     }
@@ -353,4 +353,20 @@ public class ConfiguratorTest {
         m_configurator.stop();
         FileUtils.removeDirectoryWithContent(m_configDir);
     }
+
+    /**
+     * Renames a given source file to a new destination file, using Commons-IO.
+     * <p>This avoids the problem mentioned in ACE-155.</p>
+     * 
+     * @param source the file to rename;
+     * @param dest the file to rename to.
+     */
+    private void renameFile(File source, File dest) {
+        try {
+            org.apache.commons.io.FileUtils.moveFile(source, dest);
+        }
+        catch (IOException e) {
+            throw new RuntimeException("Failed to rename file!", e);
+        }
+    }
 }

Modified: ace/trunk/ace-deployment-verifier/pom.xml
URL: http://svn.apache.org/viewvc/ace/trunk/ace-deployment-verifier/pom.xml?rev=1306176&r1=1306175&r2=1306176&view=diff
==============================================================================
--- ace/trunk/ace-deployment-verifier/pom.xml (original)
+++ ace/trunk/ace-deployment-verifier/pom.xml Wed Mar 28 05:35:42 2012
@@ -46,7 +46,8 @@
             org.apache.ace.deployment.verifier;version=${project.version},org.osgi.service.log,org.osgi.framework.wiring
         </export.package>
         <private.package>
-            org.apache.felix.*,org.apache.ace.deployment.verifier.impl.*
+            org.apache.felix.*,
+            org.apache.ace.deployment.verifier.impl.*
         </private.package>
         <import.package>
             org.osgi.framework;version="[1.5,2)",*

Modified: ace/trunk/ace-obr-metadata/pom.xml
URL: http://svn.apache.org/viewvc/ace/trunk/ace-obr-metadata/pom.xml?rev=1306176&r1=1306175&r2=1306176&view=diff
==============================================================================
--- ace/trunk/ace-obr-metadata/pom.xml (original)
+++ ace/trunk/ace-obr-metadata/pom.xml Wed Mar 28 05:35:42 2012
@@ -58,10 +58,12 @@
         <private.package>
             org.apache.ace.obr.metadata.bindex
         </private.package>
+        <embed.dependency>
+        	artifactId=kxml2
+        </embed.dependency>
         <bundle.activator>
             org.apache.ace.obr.metadata.bindex.Activator
         </bundle.activator>
-        <embed.dependency>kxml2</embed.dependency>
     </properties>
 
     <dependencies>

Modified: ace/trunk/ace-obr-metadata/src/main/java/org/apache/ace/obr/metadata/bindex/BIndexMetadataGenerator.java
URL: http://svn.apache.org/viewvc/ace/trunk/ace-obr-metadata/src/main/java/org/apache/ace/obr/metadata/bindex/BIndexMetadataGenerator.java?rev=1306176&r1=1306175&r2=1306176&view=diff
==============================================================================
--- ace/trunk/ace-obr-metadata/src/main/java/org/apache/ace/obr/metadata/bindex/BIndexMetadataGenerator.java (original)
+++ ace/trunk/ace-obr-metadata/src/main/java/org/apache/ace/obr/metadata/bindex/BIndexMetadataGenerator.java Wed Mar 28 05:35:42 2012
@@ -18,8 +18,13 @@
  */
 package org.apache.ace.obr.metadata.bindex;
 
+import java.io.Closeable;
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
 import java.io.IOException;
+import java.nio.channels.FileChannel;
+
 import org.apache.ace.obr.metadata.MetadataGenerator;
 import org.osgi.impl.bundle.bindex.Index;
 import org.osgi.service.log.LogService;
@@ -37,37 +42,122 @@ public class BIndexMetadataGenerator imp
             File index = new File(directory, INDEX_FILENAME + INDEX_EXTENSION);
             try {
                 tempIndex = File.createTempFile("repo", INDEX_EXTENSION, directory);
-                Index.main(new String[] {"-q", "-a", "-r", tempIndex.getAbsolutePath(), directory.getAbsolutePath()});
-                // TODO: try to move the index file to it's final location, this can fail if the target
-                // file was not released by a third party before we were called (not all platforms support reading and writing
-                // to a file at the same time), for now we will try 10 times and throw an IOException if the move has not
-                // succeeded by then.
-                boolean renameOK = false;
-                int attempts = 0;
-                while(!renameOK && (attempts < 10)) {
-                    index.delete();
-                    renameOK = tempIndex.renameTo(index);
-                    if (!renameOK) {
-                        attempts++;
-                        Thread.sleep(1000);
-                    }
-                }
-                if (!renameOK) {
-                    m_log.log(LogService.LOG_ERROR, "Unable to move new repository index to it's final location.");
-                    throw new IOException("Could not move temporary index file (" + tempIndex.getAbsolutePath() + ") to it's final location (" + index.getAbsolutePath() + ")");
-                }
+                Index.main(new String[] { "-q", "-a", "-r", tempIndex.getAbsolutePath(), directory.getAbsolutePath() });
+                renameFile(tempIndex, index);
             }
             catch (IOException e) {
-                m_log.log(LogService.LOG_ERROR, "Unable to create temporary file for new repository index.", e);
+                if (m_log != null) {
+                    m_log.log(LogService.LOG_ERROR, "Unable to create temporary file for new repository index.", e);
+                }
                 throw e;
             }
             catch (InterruptedException e) {
-                m_log.log(LogService.LOG_ERROR, "Waiting for next attempt to move temporary repository index failed.", e);
+                if (m_log != null) {
+                    m_log.log(LogService.LOG_ERROR, "Waiting for next attempt to move temporary repository index failed.", e);
+                }
+                // Make sure the thread's administration remains correct...
+                Thread.currentThread().interrupt();
             }
             catch (Exception e) {
-                m_log.log(LogService.LOG_ERROR, "Failed to generate new repository index.", e);
+                if (m_log != null) {
+                    m_log.log(LogService.LOG_ERROR, "Failed to generate new repository index.", e);
+                }
                 throw new IOException("Failed to generate new repository index. + (" + e.getMessage() + ")");
             }
         }
     }
+
+    /**
+     * Renames a given source file to a new destination file, using Commons-IO.
+     * <p>This avoids the problem mentioned in ACE-155.</p>
+     * 
+     * @param source the file to rename;
+     * @param dest the file to rename to.
+     */
+    private void renameFile(File source, File dest) throws IOException, InterruptedException {
+        boolean renameOK = false;
+        int attempts = 0;
+        while (!renameOK && (attempts++ < 10)) {
+            try {
+                renameOK = moveFile(source, dest);
+            }
+            catch (IOException e) {
+                // In all other cases, we assume the source file is still locked and cannot be removed;
+                Thread.sleep(1000);
+            }
+        }
+
+        if (!renameOK) {
+            if (m_log != null) {
+                m_log.log(LogService.LOG_ERROR, "Unable to move new repository index to it's final location.");
+            }
+            throw new IOException("Could not move temporary index file (" + source.getAbsolutePath() + ") to it's final location (" + dest.getAbsolutePath() + ")");
+        }
+    }
+
+    /**
+     * Moves a given source file to a destination location, effectively resulting in a rename.
+     * 
+     * @param source the source file to move;
+     * @param dest the destination file to move the file to.
+     * @return <code>true</code> if the move succeeded.
+     * @throws IOException in case of I/O problems.
+     */
+    private boolean moveFile(File source, File dest) throws IOException {
+        final int bufferSize = 1024 * 1024; // 1MB
+
+        FileInputStream fis = null;
+        FileOutputStream fos = null;
+        FileChannel input = null;
+        FileChannel output = null;
+
+        try {
+            fis = new FileInputStream(source);
+            input = fis.getChannel();
+
+            fos = new FileOutputStream(dest);
+            output = fos.getChannel();
+
+            long size = input.size();
+            long pos = 0;
+            while (pos < size) {
+                pos += output.transferFrom(input, pos, Math.min(size - pos, bufferSize));
+            }
+        }
+        finally {
+            closeQuietly(fos);
+            closeQuietly(fis);
+            closeQuietly(output);
+            closeQuietly(input);
+        }
+
+        if (source.length() != dest.length()) {
+            throw new IOException("Failed to move file! Not all contents from '" + source + "' copied to '" + dest + "'!");
+        }
+
+        dest.setLastModified(source.lastModified());
+
+        if (!source.delete()) {
+            dest.delete();
+            throw new IOException("Failed to move file! Source file (" + source + ") locked?");
+        }
+
+        return true;
+    }
+
+    /**
+     * Safely closes a given resource, ignoring any I/O exceptions that might occur by this.
+     * 
+     * @param resource the resource to close, can be <code>null</code>.
+     */
+    private void closeQuietly(Closeable resource) {
+        try {
+            if (resource != null) {
+                resource.close();
+            }
+        }
+        catch (IOException e) {
+            // Ignored...
+        }
+    }
 }
\ No newline at end of file

Modified: ace/trunk/ace-obr-storage/pom.xml
URL: http://svn.apache.org/viewvc/ace/trunk/ace-obr-storage/pom.xml?rev=1306176&r1=1306175&r2=1306176&view=diff
==============================================================================
--- ace/trunk/ace-obr-storage/pom.xml (original)
+++ ace/trunk/ace-obr-storage/pom.xml Wed Mar 28 05:35:42 2012
@@ -52,9 +52,7 @@
         </import.package>
         <private.package>
             org.apache.ace.obr.storage.file,
-            org.apache.ace.obr.storage.file.constants,
-            org.apache.commons.io,
-            org.apache.commons.io.*
+            org.apache.ace.obr.storage.file.constants
         </private.package>
         <bundle.activator>
             org.apache.ace.obr.storage.file.Activator
@@ -82,10 +80,5 @@
             <groupId>org.apache.ace</groupId>
             <artifactId>org.apache.ace.util</artifactId>
         </dependency>
-        <dependency>
-            <groupId>commons-io</groupId>
-            <artifactId>commons-io</artifactId>
-            <version>2.0.1</version>
-        </dependency>
     </dependencies>
 </project>

Modified: ace/trunk/ace-obr-storage/src/main/java/org/apache/ace/obr/storage/file/BundleFileStore.java
URL: http://svn.apache.org/viewvc/ace/trunk/ace-obr-storage/src/main/java/org/apache/ace/obr/storage/file/BundleFileStore.java?rev=1306176&r1=1306175&r2=1306176&view=diff
==============================================================================
--- ace/trunk/ace-obr-storage/src/main/java/org/apache/ace/obr/storage/file/BundleFileStore.java (original)
+++ ace/trunk/ace-obr-storage/src/main/java/org/apache/ace/obr/storage/file/BundleFileStore.java Wed Mar 28 05:35:42 2012
@@ -18,11 +18,13 @@
  */
 package org.apache.ace.obr.storage.file;
 
+import java.io.Closeable;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.channels.FileChannel;
 import java.util.Dictionary;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -30,7 +32,6 @@ import java.util.concurrent.ConcurrentHa
 import org.apache.ace.obr.metadata.MetadataGenerator;
 import org.apache.ace.obr.storage.BundleStore;
 import org.apache.ace.obr.storage.file.constants.OBRFileStoreConstants;
-import org.apache.commons.io.FileUtils;
 import org.osgi.service.cm.ConfigurationException;
 import org.osgi.service.cm.ManagedService;
 import org.osgi.service.log.LogService;
@@ -40,56 +41,60 @@ import org.osgi.service.log.LogService;
  * the repository.xml should be retrievable from that path (which will internally be converted to an absolute path).
  */
 public class BundleFileStore implements BundleStore, ManagedService {
+
     private static int BUFFER_SIZE = 8 * 1024;
+    private static final String REPOSITORY_XML = "repository.xml";
+
+    private final Map<String, Long> m_foundFiles = new ConcurrentHashMap<String, Long>();
+    private final Object m_dirLock = new Object();
 
     private volatile MetadataGenerator m_metadata; /* will be injected by dependencymanager */
     private volatile LogService m_log; /* will be injected by dependencymanager */
 
-    private final Map<String, Long> m_foundFiles = new ConcurrentHashMap<String, Long>();
-    private volatile File m_dir;
+    /** protected by m_dirLock. */
+    private File m_dir;
 
-    @SuppressWarnings("unused")
-    private void start() {
-        try {
-            generateMetadata();
-        }
-        catch (IOException e) {
-            m_log.log(LogService.LOG_ERROR, "Could not generate initial meta data for bundle repository");
+    public void generateMetadata() throws IOException {
+        File dir = getWorkingDir();
+        File[] files = dir.listFiles();
+
+        m_metadata.generateMetadata(dir);
+
+        for (File current : files) {
+            m_foundFiles.put(current.getAbsolutePath(), current.lastModified() ^ current.length());
         }
     }
 
     public InputStream get(String fileName) throws IOException {
-        if (fileName.equals("repository.xml") && directoryChanged()) {
+        if (REPOSITORY_XML.equals(fileName) && directoryChanged(getWorkingDir())) {
             generateMetadata(); // might be called too often
         }
-        return new FileInputStream(new File(m_dir, fileName));
+        return new FileInputStream(createFile(fileName));
     }
 
-    public synchronized boolean put(String fileName, InputStream data) throws IOException {
-        File file = new File(m_dir, fileName);
+    public boolean put(String fileName, InputStream data) throws IOException {
+        final File file = createFile(fileName);
+
         boolean success = false;
         if (!file.exists()) {
-            File tempFile = null;
             try {
-                tempFile = File.createTempFile("obr", ".tmp");
                 // the reason for first writing to a temporary file is that we want to minimize
                 // the window where someone could be looking at a "partial" file that is still being
                 // uploaded
-                FileUtils.copyInputStreamToFile(data, tempFile);
-                FileUtils.moveFile(tempFile, file);
+                downloadToFile(data, file);
                 success = true;
             }
             catch (IOException e) {
                 // if anything goes wrong while reading from the input stream or
                 // moving the file, delete the temporary file
-                tempFile.delete();
             }
         }
         return success;
     }
 
-    public synchronized boolean remove(String fileName) throws IOException {
-        File file = new File(m_dir, fileName);
+    public boolean remove(String fileName) throws IOException {
+        File file = createFile(fileName);
+
         if (file.exists()) {
             if (file.delete()) {
                 return true;
@@ -98,12 +103,57 @@ public class BundleFileStore implements 
                 throw new IOException("Unable to delete file (" + file.getAbsolutePath() + ")");
             }
         }
+
         return false;
     }
 
+    @SuppressWarnings("unchecked")
+    public void updated(Dictionary dict) throws ConfigurationException {
+        if (dict != null) {
+            String path = (String) dict.get(OBRFileStoreConstants.FILE_LOCATION_KEY);
+            if (path == null) {
+                throw new ConfigurationException(OBRFileStoreConstants.FILE_LOCATION_KEY, "Missing property");
+            }
+
+            File newDir = new File(path);
+            File curDir = getWorkingDir();
+
+            if (!newDir.equals(curDir)) {
+                if (!newDir.exists()) {
+                    newDir.mkdirs();
+                }
+                else if (!newDir.isDirectory()) {
+                    throw new ConfigurationException(OBRFileStoreConstants.FILE_LOCATION_KEY, "Is not a directory: " + newDir);
+                }
+
+                synchronized (m_dirLock) {
+                    m_dir = newDir;
+                }
+
+                m_foundFiles.clear();
+            }
+        }
+        else {
+            // clean up after getting a null as dictionary, as the service is going to be pulled afterwards
+            m_foundFiles.clear();
+        }
+    }
+
+    /**
+     * Called by dependencymanager upon start of this component.
+     */
+    protected void start() {
+        try {
+            generateMetadata();
+        }
+        catch (IOException e) {
+            m_log.log(LogService.LOG_ERROR, "Could not generate initial meta data for bundle repository");
+        }
+    }
+
     @SuppressWarnings("boxing")
-    private boolean directoryChanged() {
-        File[] files = m_dir.listFiles();
+    private boolean directoryChanged(File dir) {
+        File[] files = dir.listFiles();
 
         // if number of files changed, create new metadata
         if (files.length != m_foundFiles.size()) {
@@ -122,40 +172,130 @@ public class BundleFileStore implements 
                 return true;
             }
         }
+
         return false;
     }
 
-    @SuppressWarnings("boxing")
-    public synchronized void generateMetadata() throws IOException {
-        File[] files = m_dir.listFiles();
-        m_metadata.generateMetadata(m_dir);
-        for (File current : files) {
-            m_foundFiles.put(current.getAbsolutePath(), current.lastModified() ^ current.length());
+    /**
+     * Downloads a given input stream to a temporary file and if done, moves it to its final location.
+     * 
+     * @param source the input stream to download;
+     * @param dest the destination to write the downloaded file to.
+     * @throws IOException in case of I/O problems.
+     */
+    private void downloadToFile(InputStream source, File dest) throws IOException {
+        File tempFile = File.createTempFile("obr", ".tmp");
+
+        FileOutputStream fos = null;
+
+        try {
+            fos = new FileOutputStream(tempFile);
+
+            int read;
+            byte[] buffer = new byte[BUFFER_SIZE];
+            while ((read = source.read(buffer)) >= 0) {
+                fos.write(buffer, 0, read);
+            }
+            fos.flush();
+
+            if (!tempFile.renameTo(dest)) {
+                if (!moveFile(tempFile, dest)) {
+                    throw new IOException("Failed to move file store to its destination!");
+                }
+            }
+        }
+        finally {
+            closeQuietly(fos);
+
+            tempFile.delete();
         }
     }
 
-    @SuppressWarnings("unchecked")
-    public synchronized void updated(Dictionary dict) throws ConfigurationException {
-        if (dict != null) {
-            String path = (String) dict.get(OBRFileStoreConstants.FILE_LOCATION_KEY);
-            if (path == null) {
-                throw new ConfigurationException(OBRFileStoreConstants.FILE_LOCATION_KEY, "Missing property");
+    /**
+     * @return the working directory of this file store.
+     */
+    private File getWorkingDir() {
+        final File dir;
+        synchronized (m_dirLock) {
+            dir = m_dir;
+        }
+        return dir;
+    }
+
+    /**
+     * Moves a given source file to a destination location, effectively resulting in a rename.
+     * 
+     * @param source the source file to move;
+     * @param dest the destination file to move the file to.
+     * @return <code>true</code> if the move succeeded.
+     * @throws IOException in case of I/O problems.
+     */
+    private boolean moveFile(File source, File dest) throws IOException {
+        final int bufferSize = 1024 * 1024; // 1MB
+
+        FileInputStream fis = null;
+        FileOutputStream fos = null;
+        FileChannel input = null;
+        FileChannel output = null;
+
+        try {
+            fis = new FileInputStream(source);
+            input = fis.getChannel();
+
+            fos = new FileOutputStream(dest);
+            output = fos.getChannel();
+
+            long size = input.size();
+            long pos = 0;
+            while (pos < size) {
+                pos += output.transferFrom(input, pos, Math.min(size - pos, bufferSize));
             }
-            File dir = new File(path);
-            if (!dir.equals(m_dir)) {
-                if (!dir.exists()) {
-                    dir.mkdirs();
-                }
-                else if (!dir.isDirectory()) {
-                    throw new ConfigurationException(OBRFileStoreConstants.FILE_LOCATION_KEY, "Is not a directory: " + dir);
-                }
-                m_dir = dir;
-                m_foundFiles.clear();
+        }
+        finally {
+            closeQuietly(fos);
+            closeQuietly(fis);
+            closeQuietly(output);
+            closeQuietly(input);
+        }
+
+        if (source.length() != dest.length()) {
+            throw new IOException("Failed to move file! Not all contents from '" + source + "' copied to '" + dest + "'!");
+        }
+
+        dest.setLastModified(source.lastModified());
+
+        if (!source.delete()) {
+            dest.delete();
+            throw new IOException("Failed to move file! Source file (" + source + ") locked?");
+        }
+
+        return true;
+    }
+
+    /**
+     * Safely closes a given resource, ignoring any I/O exceptions that might occur by this.
+     * 
+     * @param resource the resource to close, can be <code>null</code>.
+     */
+    private void closeQuietly(Closeable resource) {
+        try {
+            if (resource != null) {
+                resource.close();
             }
         }
-        else {
-            // clean up after getting a null as dictionary, as the service is going to be pulled afterwards
-            m_foundFiles.clear();
+        catch (IOException e) {
+            // Ignored...
         }
     }
+
+    /**
+     * Creates a {@link File} object with the given file name in the current working directory.
+     * 
+     * @param fileName the name of the file.
+     * @return a {@link File} object, never <code>null</code>.
+     * @see #getWorkingDir()
+     */
+    private File createFile(String fileName) {
+        return new File(getWorkingDir(), fileName);
+    }
 }
\ No newline at end of file

Modified: ace/trunk/ace-repository-impl/src/main/java/org/apache/ace/repository/impl/RepositoryImpl.java
URL: http://svn.apache.org/viewvc/ace/trunk/ace-repository-impl/src/main/java/org/apache/ace/repository/impl/RepositoryImpl.java?rev=1306176&r1=1306175&r2=1306176&view=diff
==============================================================================
--- ace/trunk/ace-repository-impl/src/main/java/org/apache/ace/repository/impl/RepositoryImpl.java (original)
+++ ace/trunk/ace-repository-impl/src/main/java/org/apache/ace/repository/impl/RepositoryImpl.java Wed Mar 28 05:35:42 2012
@@ -18,12 +18,14 @@
  */
 package org.apache.ace.repository.impl;
 
+import java.io.Closeable;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.channels.FileChannel;
 import java.util.Arrays;
 
 import org.apache.ace.range.SortedRangeSet;
@@ -44,10 +46,10 @@ import org.osgi.service.log.LogService;
 public class RepositoryImpl implements RepositoryReplication, Repository {
 
     private volatile LogService m_log; /* will be injected by dependency manager */
+    private volatile boolean m_isMaster;
 
     private final File m_tempDir;
-    private File m_dir;
-    private boolean m_isMaster;
+    private final File m_dir;
 
     /**
      * Creates a new repository.
@@ -87,11 +89,11 @@ public class RepositoryImpl implements R
         OutputStream fileStream = null;
         try {
             fileStream = new FileOutputStream(tempFile);
+            
             byte[] buffer = new byte[1024];
-            int bytes = data.read(buffer);
-            while (bytes != -1) {
+            int bytes;
+            while ((bytes = data.read(buffer)) >= 0) {
                 fileStream.write(buffer, 0, bytes);
-                bytes = data.read(buffer);
             }
         }
         catch (IOException e) {
@@ -114,13 +116,7 @@ public class RepositoryImpl implements R
         }
 
         // move temp file to final location
-        if (!tempFile.renameTo(file)) {
-            String deleteMsg = "";
-            if (!tempFile.delete()) {
-                deleteMsg = " and was unable to remove temp file " + tempFile.getAbsolutePath();
-            }
-            throw new IOException("Unable to move temp file (" + tempFile.getAbsolutePath() + ") to final location (" + file.getAbsolutePath() + ")" + deleteMsg);
-        }
+        renameFile(tempFile, file);
 
         return true;
     }
@@ -192,7 +188,108 @@ public class RepositoryImpl implements R
      *
      * @throws ConfigurationException If it was impossible to use the new configuration.
      */
-    public synchronized void updated(boolean isMaster) throws ConfigurationException {
+    public void updated(boolean isMaster) throws ConfigurationException {
         m_isMaster = isMaster;
     }
+
+    /**
+     * Renames a given source file to a new destination file.
+     * <p>
+     * This avoids the problem mentioned in ACE-155.<br/>
+     * The moveFile method from Commons-IO is not used, as it would mean that
+     * we need to include this JAR in several placed for only a few lines of
+     * code.
+     * </p>
+     * 
+     * @param source the file to rename;
+     * @param dest the file to rename to.
+     */
+    private void renameFile(File source, File dest) throws IOException {
+        boolean renameOK = false;
+        int attempts = 0;
+        while (!renameOK && (attempts++ < 10)) {
+            try {
+                renameOK = source.renameTo(dest);
+                if (!renameOK) {
+                    renameOK = moveFile(source, dest);
+                }
+            }
+            catch (IOException e) {
+                // In all other cases, we assume the source file is still locked and cannot be removed;
+            }
+        }
+
+        if (!renameOK) {
+            if (m_log != null) {
+                m_log.log(LogService.LOG_ERROR, "Unable to move new repository file to it's final location.");
+            }
+            throw new IOException("Could not move temporary file (" + source.getAbsolutePath() + ") to it's final location (" + dest.getAbsolutePath() + ")");
+        }
+    }
+
+    /**
+     * Moves a given source file to a destination location, effectively resulting in a rename.
+     * 
+     * @param source the source file to move;
+     * @param dest the destination file to move the file to.
+     * @return <code>true</code> if the move succeeded.
+     * @throws IOException in case of I/O problems.
+     */
+    private boolean moveFile(File source, File dest) throws IOException {
+        final int bufferSize = 1024 * 1024; // 1MB
+
+        FileInputStream fis = null;
+        FileOutputStream fos = null;
+        FileChannel input = null;
+        FileChannel output = null;
+        
+        try {
+            fis = new FileInputStream(source);
+            input = fis.getChannel();
+
+            fos = new FileOutputStream(dest);
+            output = fos.getChannel();
+
+            long size = input.size();
+            long pos = 0;
+            while (pos < size) {
+                pos += output.transferFrom(input, pos, Math.min(size - pos, bufferSize));
+            }
+        }
+        finally {
+            closeQuietly(fos);
+            closeQuietly(fis);
+            closeQuietly(output);
+            closeQuietly(input);
+        }
+
+        if (source.length() != dest.length()) {
+            throw new IOException("Failed to move file! Not all contents from '" + source + "' copied to '" + dest + "'!");
+        }
+
+        dest.setLastModified(source.lastModified());
+
+        if (!source.delete()) {
+            dest.delete();
+            throw new IOException("Failed to move file! Source file (" + source + ") locked?");
+        }
+
+        return true;
+    }
+
+    /**
+     * Safely closes a given resource, ignoring any I/O exceptions that might occur by this.
+     * 
+     * @param resource the resource to close, can be <code>null</code>.
+     */
+    private void closeQuietly(Closeable resource) {
+        try {
+            if (resource != null) {
+                resource.close();
+            }
+        }
+        catch (IOException e) {
+            // Ignored...
+        }
+    }
 }
\ No newline at end of file

Modified: ace/trunk/pom/pom.xml
URL: http://svn.apache.org/viewvc/ace/trunk/pom/pom.xml?rev=1306176&r1=1306175&r2=1306176&view=diff
==============================================================================
--- ace/trunk/pom/pom.xml (original)
+++ ace/trunk/pom/pom.xml Wed Mar 28 05:35:42 2012
@@ -136,6 +136,7 @@
         <commons-collections.version>3.2.1</commons-collections.version>
         <commons-lang.version>2.4</commons-lang.version>
         <commons-logging.version>1.1.1</commons-logging.version>
+        <commons-io.version>2.0.1</commons-io.version>
         <easymock.version>2.5.2</easymock.version>
         <felix.configadmin.version>1.2.8</felix.configadmin.version>
         <felix.dependencymanager.version>3.0.0</felix.dependencymanager.version>
@@ -695,6 +696,11 @@
                 <artifactId>commons-logging</artifactId>
                 <version>${commons-logging.version}</version>
             </dependency>
+            <dependency>
+                <groupId>commons-io</groupId>
+                <artifactId>commons-io</artifactId>
+                <version>${commons-io.version}</version>
+            </dependency>
 
             <!-- OPS4J -->
             <dependency>