jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "angela (JIRA)" <j...@apache.org>
Subject [jira] Updated: (JCR-1458) Avoid silent closes
Date Tue, 18 May 2010 10:47:42 GMT

     [ 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.

View raw message