You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ra...@apache.org on 2014/07/04 04:20:00 UTC

svn commit: r1607774 - in /zookeeper/trunk: ./ src/java/main/org/apache/zookeeper/common/ src/java/main/org/apache/zookeeper/server/quorum/ src/java/test/org/apache/zookeeper/common/

Author: rakeshr
Date: Fri Jul  4 02:19:59 2014
New Revision: 1607774

URL: http://svn.apache.org/r1607774
Log:
ZOOKEEPER-1835. dynamic configuration file renaming fails on Windows(Bruno Freudensprung via rakeshr)

Added:
    zookeeper/trunk/src/java/main/org/apache/zookeeper/common/AtomicFileWritingIdiom.java
    zookeeper/trunk/src/java/test/org/apache/zookeeper/common/AtomicFileWritingIdiomTest.java
Modified:
    zookeeper/trunk/CHANGES.txt
    zookeeper/trunk/src/java/main/org/apache/zookeeper/common/AtomicFileOutputStream.java
    zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
    zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java

Modified: zookeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/trunk/CHANGES.txt?rev=1607774&r1=1607773&r2=1607774&view=diff
==============================================================================
--- zookeeper/trunk/CHANGES.txt (original)
+++ zookeeper/trunk/CHANGES.txt Fri Jul  4 02:19:59 2014
@@ -678,6 +678,9 @@ BUGFIXES:
   ZOOKEEPER-1939. ReconfigRecoveryTest.testNextConfigUnreachable is
   failing (Rakesh R via phunt)
 
+  ZOOKEEPER-1835. dynamic configuration file renaming fails on Windows
+  (Bruno Freudensprung via rakeshr)
+
 IMPROVEMENTS:
 
   ZOOKEEPER-1170. Fix compiler (eclipse) warnings: unused imports,

Modified: zookeeper/trunk/src/java/main/org/apache/zookeeper/common/AtomicFileOutputStream.java
URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/main/org/apache/zookeeper/common/AtomicFileOutputStream.java?rev=1607774&r1=1607773&r2=1607774&view=diff
==============================================================================
--- zookeeper/trunk/src/java/main/org/apache/zookeeper/common/AtomicFileOutputStream.java (original)
+++ zookeeper/trunk/src/java/main/org/apache/zookeeper/common/AtomicFileOutputStream.java Fri Jul  4 02:19:59 2014
@@ -68,7 +68,7 @@ public class AtomicFileOutputStream exte
         boolean triedToClose = false, success = false;
         try {
             flush();
-            ((FileOutputStream) out).getChannel().force(true);
+            ((FileOutputStream) out).getFD().sync();
 
             triedToClose = true;
             super.close();

Added: zookeeper/trunk/src/java/main/org/apache/zookeeper/common/AtomicFileWritingIdiom.java
URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/main/org/apache/zookeeper/common/AtomicFileWritingIdiom.java?rev=1607774&view=auto
==============================================================================
--- zookeeper/trunk/src/java/main/org/apache/zookeeper/common/AtomicFileWritingIdiom.java (added)
+++ zookeeper/trunk/src/java/main/org/apache/zookeeper/common/AtomicFileWritingIdiom.java Fri Jul  4 02:19:59 2014
@@ -0,0 +1,89 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.common;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+
+/*
+ *  Used to perform an atomic write into a file.
+ *  If there is a failure in the middle of the writing operation, 
+ *  the original file (if it exists) is left intact.
+ *  Based on the org.apache.zookeeper.server.quorum.QuorumPeer.writeLongToFile(...) idiom
+ *  using the HDFS AtomicFileOutputStream class.
+ */
+public class AtomicFileWritingIdiom {
+
+    public static interface OutputStreamStatement {
+
+        public void write(OutputStream os) throws IOException;
+
+    }
+
+    public static interface WriterStatement {
+
+        public void write(Writer os) throws IOException;
+
+    }
+
+    public AtomicFileWritingIdiom(File targetFile, OutputStreamStatement osStmt)  throws IOException {
+        this(targetFile, osStmt, null);
+    }
+
+    public AtomicFileWritingIdiom(File targetFile, WriterStatement wStmt)  throws IOException {
+        this(targetFile, null, wStmt);
+    }
+
+    private AtomicFileWritingIdiom(File targetFile, OutputStreamStatement osStmt, WriterStatement wStmt)  throws IOException {
+        AtomicFileOutputStream out = null;
+        boolean error = true;
+        try {
+            out = new AtomicFileOutputStream(targetFile);
+            if (wStmt == null) {
+                // execute output stream operation
+                osStmt.write(out);
+            } else {
+                BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(out));
+                // execute writer operation and flush
+                wStmt.write(bw);
+                bw.flush();
+            }
+            out.flush();
+            // everything went ok
+            error = false;
+        } finally {
+            // nothing interesting to do if out == null
+            if (out != null) {
+                if (error) {
+                    // worst case here the tmp file/resources(fd) are not cleaned up
+                    // and the caller will be notified (IOException)
+                    out.abort();
+                } else {
+                    // if the close operation (rename) fails we'll get notified.
+                    // worst case the tmp file may still exist
+                    IOUtils.closeStream(out);
+                }
+            }
+        }
+    }
+
+}

Modified: zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java?rev=1607774&r1=1607773&r2=1607774&view=diff
==============================================================================
--- zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java (original)
+++ zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java Fri Jul  4 02:19:59 2014
@@ -28,6 +28,7 @@ import java.io.StringWriter;
 import java.io.OutputStreamWriter;
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.io.Writer;
 import java.net.DatagramPacket;
 import java.net.DatagramSocket;
 import java.net.InetSocketAddress;
@@ -46,7 +47,8 @@ import java.util.Set;
 
 import org.apache.zookeeper.KeeperException.NoNodeException;
 import org.apache.zookeeper.ZooDefs;
-import org.apache.zookeeper.common.AtomicFileOutputStream;
+import org.apache.zookeeper.common.AtomicFileWritingIdiom;
+import org.apache.zookeeper.common.AtomicFileWritingIdiom.WriterStatement;
 import org.apache.zookeeper.common.HostNameUtils;
 import org.apache.zookeeper.jmx.MBeanRegistry;
 import org.apache.zookeeper.jmx.ZKMBeanInfo;
@@ -1488,30 +1490,14 @@ public class QuorumPeer extends ZooKeepe
 	 * @param value the long value to write to the named file
 	 * @throws IOException if the file cannot be written atomically
 	 */
-    private void writeLongToFile(String name, long value) throws IOException {
+    private void writeLongToFile(String name, final long value) throws IOException {
         File file = new File(logFactory.getSnapDir(), name);
-        AtomicFileOutputStream out = new AtomicFileOutputStream(file);
-        BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(out));
-        boolean aborted = false;
-        try {
-            bw.write(Long.toString(value));
-            bw.flush();
-            
-            out.flush();
-        } catch (IOException e) {
-            LOG.error("Failed to write new file " + file, e);
-            // worst case here the tmp file/resources(fd) are not cleaned up
-            //   and the caller will be notified (IOException)
-            aborted = true;
-            out.abort();
-            throw e;
-        } finally {
-            if (!aborted) {
-                // if the close operation (rename) fails we'll get notified.
-                // worst case the tmp file may still exist
-                out.close();
+        new AtomicFileWritingIdiom(file, new WriterStatement() {
+            @Override
+            public void write(Writer bw) throws IOException {
+                bw.write(Long.toString(value));
             }
-        }
+        });
     }
 
     public long getCurrentEpoch() throws IOException {

Modified: zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java?rev=1607774&r1=1607773&r2=1607774&view=diff
==============================================================================
--- zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java (original)
+++ zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java Fri Jul  4 02:19:59 2014
@@ -26,6 +26,8 @@ import java.io.FileOutputStream;
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.io.OutputStream;
+import java.io.Writer;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.util.Collections;
@@ -36,8 +38,9 @@ import java.util.Map.Entry;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.MDC;
-
-
+import org.apache.zookeeper.common.AtomicFileWritingIdiom;
+import org.apache.zookeeper.common.AtomicFileWritingIdiom.OutputStreamStatement;
+import org.apache.zookeeper.common.AtomicFileWritingIdiom.WriterStatement;
 import org.apache.zookeeper.server.ZooKeeperServer;
 import org.apache.zookeeper.server.quorum.QuorumPeer.LearnerType;
 import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer;
@@ -299,74 +302,55 @@ public class QuorumPeerConfig {
      * @param qv
      */
     public static void writeDynamicConfig(String dynamicConfigFilename, String configFileStr, 
-            boolean configBackwardCompatibilityMode, QuorumVerifier qv) throws IOException {                             
-        FileOutputStream outConfig = null;
-       try {
-           byte b[] = qv.toString().getBytes();                                            
-           if (configBackwardCompatibilityMode) {
-               dynamicConfigFilename = configFileStr + ".dynamic";
-           }
-           String tmpFilename = dynamicConfigFilename + ".tmp";
-           outConfig = new FileOutputStream(tmpFilename);
-           
-           outConfig.write(b);
-           outConfig.close();
-           File curFile = new File(dynamicConfigFilename);
-           File tmpFile = new File(tmpFilename);
-           if (!tmpFile.renameTo(curFile)) {
-               throw new IOException("renaming " + tmpFile.toString() + " to " + curFile.toString() + " failed!");
-           }
-       } finally{
-           if (outConfig!=null) { 
-               outConfig.close();
-           }
-       }
-       // the following is for users who run without a dynamic config file (old config file)
-       // if the configuration changes (reconfiguration executes), we create a dynamic config
-       // file, remove all the dynamic definitions from the config file and add a pointer
-       // to the config file. The dynamic config file's name will be the same as the config file's
-       // with ".dynamic" appended to it
+            boolean configBackwardCompatibilityMode, final QuorumVerifier qv) throws IOException {
+
+        if (configBackwardCompatibilityMode) {
+            dynamicConfigFilename = configFileStr + ".dynamic";
+        }
+        final String actualDynamicConfigFilename = dynamicConfigFilename;
+        new AtomicFileWritingIdiom(new File(actualDynamicConfigFilename), new OutputStreamStatement() {
+            @Override
+            public void write(OutputStream outConfig) throws IOException {
+                byte b[] = qv.toString().getBytes();
+                outConfig.write(b);
+            }
+        });
+        // the following is for users who run without a dynamic config file (old config file)
+        // if the configuration changes (reconfiguration executes), we create a dynamic config
+        // file, remove all the dynamic definitions from the config file and add a pointer
+        // to the config file. The dynamic config file's name will be the same as the config file's
+        // with ".dynamic" appended to it
        
         if (configBackwardCompatibilityMode) {
-           BufferedWriter out = null;
-               try {
-                   File configFile = (new VerifyingFileFactory.Builder(LOG)
-                       .warnForRelativePath()
-                       .failForNonExistingPath()
-                       .build()).create(configFileStr);
-                       
-                   Properties cfg = new Properties();
-                   FileInputStream in = new FileInputStream(configFile);
-                   try {
-                       cfg.load(in);
-                   } finally {
-                       in.close();
-                   }
-                   String tmpFilename = configFileStr + ".tmp";                    
-                   FileWriter fstream = new FileWriter(tmpFilename);
-                   out = new BufferedWriter(fstream);                 
+               File configFile = (new VerifyingFileFactory.Builder(LOG)
+                   .warnForRelativePath()
+                   .failForNonExistingPath()
+                   .build()).create(configFileStr);
                    
-                   for (Entry<Object, Object> entry : cfg.entrySet()) {
-                       String key = entry.getKey().toString().trim();
-                       String value = entry.getValue().toString().trim();    
-                       if (!key.startsWith("server.") && !key.startsWith("group") 
-                               && !key.startsWith("weight") && !key.equals("clientPort") && !key.equals("clientPortAddress")){
-                           out.write(key.concat("=").concat(value).concat("\n"));
-                       }
-                   }                      
-                   out.write("dynamicConfigFile=".concat(dynamicConfigFilename).concat("\n"));
-                   out.close();
-                   File tmpFile = new File(tmpFilename);
-                   if (!tmpFile.renameTo(configFile)) {
-                       throw new IOException("renaming " + tmpFile.toString() + " to " + configFile.toString() + " failed!");
-                   }
-               } finally{
-                   if (out!=null) {
-                           out.close();
-                   }
+               final Properties cfg = new Properties();
+               FileInputStream in = new FileInputStream(configFile);
+               try {
+                   cfg.load(in);
+               } finally {
+                   in.close();
                }
-           }
-   } 
+               new AtomicFileWritingIdiom(new File(configFileStr), new WriterStatement() {
+                   @Override
+                   public void write(Writer out) throws IOException {
+                       for (Entry<Object, Object> entry : cfg.entrySet()) {
+                           String key = entry.getKey().toString().trim();
+                           String value = entry.getValue().toString().trim();    
+                           if (!key.startsWith("server.") && !key.startsWith("group") 
+                                   && !key.startsWith("weight") && !key.equals("clientPort") && !key.equals("clientPortAddress")){
+                               out.write(key.concat("=").concat(value).concat("\n"));
+                           }
+                       }                      
+                       out.write("dynamicConfigFile=".concat(actualDynamicConfigFilename).concat("\n"));
+                    
+                   }
+               });
+          }
+    } 
     public static void deleteFile(String filename){        
        File f = new File(filename);
        if (f.exists()) {

Added: zookeeper/trunk/src/java/test/org/apache/zookeeper/common/AtomicFileWritingIdiomTest.java
URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/test/org/apache/zookeeper/common/AtomicFileWritingIdiomTest.java?rev=1607774&view=auto
==============================================================================
--- zookeeper/trunk/src/java/test/org/apache/zookeeper/common/AtomicFileWritingIdiomTest.java (added)
+++ zookeeper/trunk/src/java/test/org/apache/zookeeper/common/AtomicFileWritingIdiomTest.java Fri Jul  4 02:19:59 2014
@@ -0,0 +1,360 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.common;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.Writer;
+
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.common.AtomicFileWritingIdiom.OutputStreamStatement;
+import org.apache.zookeeper.common.AtomicFileWritingIdiom.WriterStatement;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class AtomicFileWritingIdiomTest extends ZKTestCase {
+
+    private static File tmpdir;
+
+    @BeforeClass
+    public static void createTmpDir() {
+        tmpdir = new File("build/test/tmp");
+        tmpdir.mkdirs();
+    }
+
+    @Test
+    public void testOutputStreamSuccess() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        createFile(target, "before");
+        assertEquals("before", getContent(target));
+        new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
+            @Override
+            public void write(OutputStream os) throws IOException {
+                os.write("after".getBytes("ASCII"));
+                assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+            }
+        });
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        // content changed
+        assertEquals("after", getContent(target));
+        target.delete();
+    }
+
+    @Test
+    public void testWriterSuccess() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        createFile(target, "before");
+        assertEquals("before", getContent(target));
+        new AtomicFileWritingIdiom(target, new WriterStatement() {
+            @Override
+            public void write(Writer os) throws IOException {
+                os.write("after");
+                assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+            }
+        });
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        // content changed
+        assertEquals("after", getContent(target));
+        target.delete();
+    }
+
+    @Test
+    public void testOutputStreamFailure() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        createFile(target, "before");
+        assertEquals("before", getContent(target));
+        boolean exception = false;
+        try {
+            new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
+                @Override
+                public void write(OutputStream os) throws IOException {
+                    os.write("after".getBytes("ASCII"));
+                    os.flush();
+                    assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+                    throw new RuntimeException();
+                }
+            });
+        } catch (RuntimeException ex) {
+            exception = true;
+        }
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        assertTrue("should have raised an exception", exception);
+        // content preserved
+        assertEquals("before", getContent(target));
+        target.delete();
+    }
+
+    @Test
+    public void testWriterFailure() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        createFile(target, "before");
+        assertEquals("before", getContent(target));
+        boolean exception = false;
+        try {
+            new AtomicFileWritingIdiom(target, new WriterStatement() {
+                @Override
+                public void write(Writer os) throws IOException {
+                    os.write("after");
+                    os.flush();
+                    assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+                    throw new RuntimeException();
+                }
+            });
+        } catch (RuntimeException ex) {
+            exception = true;
+        }
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        assertTrue("should have raised an exception", exception);
+        // content preserved
+        assertEquals("before", getContent(target));
+        target.delete();
+    }
+
+    @Test
+    public void testOutputStreamFailureIOException() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        createFile(target, "before");
+        assertEquals("before", getContent(target));
+        boolean exception = false;
+        try {
+            new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
+                @Override
+                public void write(OutputStream os) throws IOException {
+                    os.write("after".getBytes("ASCII"));
+                    os.flush();
+                    assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+                    throw new IOException();
+                }
+            });
+        } catch (IOException ex) {
+            exception = true;
+        }
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        assertTrue("should have raised an exception", exception);
+        // content preserved
+        assertEquals("before", getContent(target));
+        target.delete();
+    }
+
+    @Test
+    public void testWriterFailureIOException() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        createFile(target, "before");
+        assertEquals("before", getContent(target));
+        boolean exception = false;
+        try {
+            new AtomicFileWritingIdiom(target, new WriterStatement() {
+                @Override
+                public void write(Writer os) throws IOException {
+                    os.write("after");
+                    os.flush();
+                    assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+                    throw new IOException();
+                }
+            });
+        } catch (IOException ex) {
+            exception = true;
+        }
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        assertTrue("should have raised an exception", exception);
+        // content preserved
+        assertEquals("before", getContent(target));
+        target.delete();
+    }
+
+    @Test
+    public void testOutputStreamFailureError() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        createFile(target, "before");
+        assertEquals("before", getContent(target));
+        boolean exception = false;
+        try {
+            new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
+                @Override
+                public void write(OutputStream os) throws IOException {
+                    os.write("after".getBytes("ASCII"));
+                    os.flush();
+                    assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+                    throw new Error();
+                }
+            });
+        } catch (Error ex) {
+            exception = true;
+        }
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        assertTrue("should have raised an exception", exception);
+        // content preserved
+        assertEquals("before", getContent(target));
+        target.delete();
+    }
+
+    @Test
+    public void testWriterFailureError() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        createFile(target, "before");
+        assertEquals("before", getContent(target));
+        boolean exception = false;
+        try {
+            new AtomicFileWritingIdiom(target, new WriterStatement() {
+                @Override
+                public void write(Writer os) throws IOException {
+                    os.write("after");
+                    os.flush();
+                    assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+                    throw new Error();
+                }
+            });
+        } catch (Error ex) {
+            exception = true;
+        }
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        assertTrue("should have raised an exception", exception);
+        // content preserved
+        assertEquals("before", getContent(target));
+        target.delete();
+    }
+
+    // ************** target file does not exist
+
+    @Test
+    public void testOutputStreamSuccessNE() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        target.delete();
+        assertFalse("file should not exist", target.exists());
+        new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
+            @Override
+            public void write(OutputStream os) throws IOException {
+                os.write("after".getBytes("ASCII"));
+                assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+            }
+        });
+        // content changed
+        assertEquals("after", getContent(target));
+        target.delete();
+    }
+
+    @Test
+    public void testWriterSuccessNE() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        target.delete();
+        assertFalse("file should not exist", target.exists());
+        new AtomicFileWritingIdiom(target, new WriterStatement() {
+            @Override
+            public void write(Writer os) throws IOException {
+                os.write("after");
+                assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+            }
+        });
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        // content changed
+        assertEquals("after", getContent(target));
+        target.delete();
+    }
+
+    @Test
+    public void testOutputStreamFailureNE() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        target.delete();
+        assertFalse("file should not exist", target.exists());
+        boolean exception = false;
+        try {
+            new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
+                @Override
+                public void write(OutputStream os) throws IOException {
+                    os.write("after".getBytes("ASCII"));
+                    os.flush();
+                    assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+                    throw new RuntimeException();
+                }
+            });
+        } catch (RuntimeException ex) {
+            exception = true;
+        }
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        assertTrue("should have raised an exception", exception);
+        // file should not exist
+        assertFalse("file should not exist", target.exists());
+    }
+
+    @Test
+    public void testWriterFailureNE() throws IOException {
+        File target = new File(tmpdir, "target.txt");
+        final File tmp = new File(tmpdir, "target.txt.tmp");
+        target.delete();
+        assertFalse("file should not exist", target.exists());
+        boolean exception = false;
+        try {
+            new AtomicFileWritingIdiom(target, new WriterStatement() {
+                @Override
+                public void write(Writer os) throws IOException {
+                    os.write("after");
+                    os.flush();
+                    assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
+                    throw new RuntimeException();
+                }
+            });
+        } catch (RuntimeException ex) {
+            exception = true;
+        }
+        assertFalse("tmp file should have been deleted", tmp.exists());
+        assertTrue("should have raised an exception", exception);
+        // file should not exist
+        assertFalse("file should not exist", target.exists());
+    }
+
+    private String getContent(File file, String encoding) throws IOException {
+        StringBuilder result = new StringBuilder();
+        FileInputStream fis = new FileInputStream(file);
+        byte[] b = new byte[20];
+        int nb;
+        while ((nb = fis.read(b)) != -1) {
+               result.append(new String(b, 0, nb, encoding));
+        }
+        fis.close();
+        return result.toString();
+    }
+
+    private String getContent(File file) throws IOException {
+        return getContent(file, "ASCII");
+    }
+
+    private void createFile(File file, String content) throws IOException {
+        FileOutputStream fos = new FileOutputStream(file);
+        fos.write(content.getBytes("ASCII"));
+        fos.close();
+    }
+
+}