Mailing-List: contact issues-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: issues@commons.apache.org
Date: Tue, 28 Feb 2012 10:15:49 +0000 (UTC)
From: "Laurent Michelet (Issue Comment Edited) (JIRA)" <jira@apache.org>
To: issues@commons.apache.org
Message-ID: 
 <586667839.27387.1330424149266.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] [Issue Comment Edited] (CONFIGURATION-136) Reloading may
 corrupt the configuration
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ 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:15 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 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}
                
      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 {
			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
			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
			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