Mailing-List: contact dev-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@jackrabbit.apache.org
Message-ID: <26603932.102901274179662244.JavaMail.jira@thor>
Date: Tue, 18 May 2010 06:47:42 -0400 (EDT)
From: "angela (JIRA)" <jira@apache.org>
To: dev@jackrabbit.apache.org
Subject: [jira] Updated: (JCR-1458) Avoid silent closes
In-Reply-To: <1536805816.1204794901743.JavaMail.jira@brutus>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


     [ https://issues.apache.org/jira/browse/JCR-1458?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

angela updated JCR-1458:
------------------------

    Component/s: jackrabbit-core

> Avoid silent closes
> -------------------
>
>                 Key: JCR-1458
>                 URL: https://issues.apache.org/jira/browse/JCR-1458
>             Project: Jackrabbit Content Repository
>          Issue Type: Improvement
>          Components: jackrabbit-core
>            Reporter: Jukka Zitting
>            Priority: Minor
>
> Here's a typical pattern in Jackrabbit code:
>     OutputStream stream = null;
>     try {
>         stream = ...; // create stream
>         // use stream
>     } catch (IOException e) {
>         throw RepositoryException("...", e);
>     } finally {
>         if (stream != null) {
>             try {
>                 stream.close();
>             } catch (IOException e) {
>                 // ignore
>             }
>         }
>     }
> (Note that I recently replaced many of the finally blocks above with a call to IOUtils.closeQuietly() as part of JCR-1395. The logic is still the same, even though the number of lines is smaller.)
> The assumption is that closing a stream will never throw an exception that should actually be taken into consideration. After all, you no longer have any use for the stream, so why care if closing it fails.
> However, if you think of scenarios where close() can potentially fail, all of them are pretty serious issues. Most importantly many OutputStreams buffer data internally, and the last bytes are only flushed out when close() is called. In such cases an IOException from close() is just as important as one from write(). InputStreams are less troublesome, but even there an error in close() can only mean that something is wrong with the system (otherwise, why would simple releasing of resources fail).
> Thus I would like to get rid of the "silent close" pattern, or at least require that the rationale for ignoring such exceptions is clearly explained. My proposal for a better code pattern is:
>     try {
>         OutputStream stream = ...; // create stream
>         try {
>             // use stream
>         } finally {
>             stream.close();
>         }
>     } catch (IOException e) {
>         throw new RepositoryException("...", e);
>     }
> The stacked try blocks add some complexity to the code, but that's the price you pay for increased reliability. If the extra indenting becomes a problem, I would just split the inner code to another method that is allowed to throw an IOException.
> Another potential issue is that an exception from close() might end up shadowing an earlier exception from the try block, but I've never seen that happen in practice and even if it does happen the end result from the client perspective is the same: an exception is thrown.
> The above reasoning applies also to database and other similar resources. If the closing of an UPDATE PreparedStatement fails, can we be sure that the statement was successfully executed? Even the failure in closing a SELECT ResultSet probably indicates some issue with the database, and I'd rather fail early with the exception than just ignore it.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.