[ https://issues.apache.org/jira/browse/CONFIGURATION-136?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218027#comment-13218027 ]
Laurent Michelet edited comment on CONFIGURATION-136 at 2/28/12 10:30 AM: -------------------------------------------------------------------------- THe problem here is that filesystem is not transactional. When an exception is raised, there is no rollback on the file which is written. A proposal of correction is to save in memory the file before any modifications. When an exception is raised we replace the current file with the original one. The 2 following patches has been tested successfully with XMLConfiguration file. {code:title=AbstractFileConfiguration.java|borderStyle=solid} /** * Save the configuration to the specified URL. This doesn't change the * source of the configuration, use setURL() if you need it. * * @param url * the URL * * @throws ConfigurationException * if an error occurs during the save operation */ public void save(URL url) throws ConfigurationException { OutputStream out = null; ByteArrayInputStream originalFile = null; try { final File file = new File(url.getFile()); // Save original file if existing if (file.exists()) { InputStream inputStreamOfOrignalFile = fileSystem .getInputStream(url); originalFile = saveOriginalFile(inputStreamOfOrignalFile); } out = fileSystem.getOutputStream(url); save(out); if (out instanceof VerifiableOutputStream) { ((VerifiableOutputStream) out).verify(); } } catch (IOException e) { // Rollback for IOException if existing if (originalFile != null) { reloadOriginalFile(url, originalFile, out); } throw new ConfigurationException("Could not save to URL " + url, e); } catch (Exception e) { // Rollback when any kind of Exception is raised if existing if (originalFile != null) { reloadOriginalFile(url, originalFile, out); } throw new ConfigurationException(e); } finally { closeSilent(out); } } /** * Save the original file before any modifications * * @param in * @return * @throws IOException * @since 1.9 */ private ByteArrayInputStream saveOriginalFile( InputStream inputStreamOfOrignalFile) throws IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); byte[] buffer = new byte[1024]; int len; while ((len = inputStreamOfOrignalFile.read(buffer)) > -1) { baos.write(buffer, 0, len); } baos.flush(); return new ByteArrayInputStream(baos.toByteArray()); } /** * Replace the current file with the original one before any modifications * * @param url * @param originalFile * @param outOfCurrentFile * @throws ConfigurationException * @since 1.9 */ private void reloadOriginalFile(URL url, ByteArrayInputStream originalFile, OutputStream outOfCurrentFile) throws ConfigurationException { if (outOfCurrentFile != null && originalFile != null) { try { int nextChar; while ((nextChar = originalFile.read()) != -1) outOfCurrentFile.write((char) nextChar); outOfCurrentFile.write('\n'); outOfCurrentFile.flush(); } catch (IOException ioe) { throw new ConfigurationException( "Could not save to URL " + url, ioe); } } } {code} was (Author: l.michelet): THe problem here is that filesystem is not transactional. When an exception is raised, there is no rollback on the file which is written. A proposal of correction is to save in memory the file before any modifications. When an exception is raised we replace the current file with the original one. The 2 previous patches has been tested successfully with XMLConfiguration file. {code:title=AbstractFileConfiguration.java|borderStyle=solid} /** * Save the configuration to the specified URL. This doesn't change the * source of the configuration, use setURL() if you need it. * * @param url * the URL * * @throws ConfigurationException * if an error occurs during the save operation */ public void save(URL url) throws ConfigurationException { OutputStream out = null; ByteArrayInputStream originalFile = null; try { final File file = new File(url.getFile()); // Save original file if existing if (file.exists()) { InputStream inputStreamOfOrignalFile = fileSystem .getInputStream(url); originalFile = saveOriginalFile(inputStreamOfOrignalFile); } out = fileSystem.getOutputStream(url); save(out); if (out instanceof VerifiableOutputStream) { ((VerifiableOutputStream) out).verify(); } } catch (IOException e) { // Rollback for IOException if existing if (originalFile != null) { reloadOriginalFile(url, originalFile, out); } throw new ConfigurationException("Could not save to URL " + url, e); } catch (Exception e) { // Rollback when any kind of Exception is raised if existing if (originalFile != null) { reloadOriginalFile(url, originalFile, out); } throw new ConfigurationException(e); } finally { closeSilent(out); } } /** * Save the original file before any modifications * * @param in * @return * @throws IOException * @since 1.9 */ private ByteArrayInputStream saveOriginalFile( InputStream inputStreamOfOrignalFile) throws IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); byte[] buffer = new byte[1024]; int len; while ((len = inputStreamOfOrignalFile.read(buffer)) > -1) { baos.write(buffer, 0, len); } baos.flush(); return new ByteArrayInputStream(baos.toByteArray()); } /** * Replace the current file with the original one before any modifications * * @param url * @param originalFile * @param outOfCurrentFile * @throws ConfigurationException * @since 1.9 */ private void reloadOriginalFile(URL url, ByteArrayInputStream originalFile, OutputStream outOfCurrentFile) throws ConfigurationException { if (outOfCurrentFile != null && originalFile != null) { try { int nextChar; while ((nextChar = originalFile.read()) != -1) outOfCurrentFile.write((char) nextChar); outOfCurrentFile.write('\n'); outOfCurrentFile.flush(); } catch (IOException ioe) { throw new ConfigurationException( "Could not save to URL " + url, ioe); } } } {code} > Reloading may corrupt the configuration > --------------------------------------- > > Key: CONFIGURATION-136 > URL: https://issues.apache.org/jira/browse/CONFIGURATION-136 > Project: Commons Configuration > Issue Type: Bug > Components: File reloading > Affects Versions: 1.1 > Reporter: nicolas de loof > Fix For: 1.9 > > Attachments: commons-configuration-1.5_patch_CONFIGURATION-136.jar, > commons-configuration-1.8_patch_CONFIGURATION-136.jar > > > Current reloading process clears current properties and load updated values > from > resource reader. If an IO error occurs (or invalid format), the configuration > gets corrupted and the application becomes unstable. > It may be better for hot-reload to put loaded values into a temporary > Properties > and replace previous values only when reloading is successful. > It may also allow to use a 'currentlty-reloading' flag in the synchronized > 'reload' block to avoid blocking threads during a reload (they could access > safelly the 'old' properties until reload is finished) -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira