Exception in finally block

Discussion in 'Java' started by Efi Merdler, Dec 2, 2006.

  1. Efi Merdler

    Efi Merdler Guest

    Hello,
    Assume I have

    BufferedWriter out = null;
    ....

    try {
    ....
    } catch (IOException e) {
    ....
    }

    finally {
    if (out != null)
    out.close();
    }

    However close in finally block throws IOException, How can I handle it
    ?
    I do not want my function to throw the exception, is this the only
    solution?

    Thanks,
    Efi
    Efi Merdler, Dec 2, 2006
    #1
    1. Advertising

  2. Efi Merdler wrote:
    > Hello,
    > Assume I have
    >
    > BufferedWriter out = null;
    > ...
    >
    > try {
    > ...
    > } catch (IOException e) {
    > ...
    > }
    >
    > finally {
    > if (out != null)
    > out.close();
    > }
    >
    > However close in finally block throws IOException, How can I handle it
    > ?
    > I do not want my function to throw the exception, is this the only
    > solution?
    >
    > Thanks,
    > Efi


    Hello,
    try use try-catch block in finally block

    For Example:

    try {
    ...
    } catch (Exception e) {
    ...
    } finally {
    try {
    ...
    } catch (Exception e) {
    ...
    }
    }

    Damian Dunajski
    Damian Dunajski, Dec 2, 2006
    #2
    1. Advertising

  3. Efi Merdler wrote:
    > Hello,
    > Assume I have
    >
    > BufferedWriter out = null;
    > ...
    >
    > try {
    > ...
    > } catch (IOException e) {
    > ...
    > }
    >
    > finally {
    > if (out != null)
    > out.close();
    > }
    >
    > However close in finally block throws IOException, How can I handle it
    > ?
    > I do not want my function to throw the exception, is this the only
    > solution?


    I think a better way to organise the code is:

    try {
    Writer rawOut = ...l;
    try {
    BufferedWriter out = new BufferedWriter(rawOut);
    ...
    out.flush();
    } finally {
    rawOut.close();
    }
    } catch (IOException exc) {
    ...
    }

    There are two problems with just closing the BufferedWriter: if the
    construction of the BufferedWriter fails you will not be able to close
    it and there is a bug in BufferedWriter.close reporting exceptions from
    flush (in that it doesn't).

    If you are doing it a lot, you may like to use the "execute-around" idiom.

    Tom Hawtin
    Thomas Hawtin, Dec 3, 2006
    #3
  4. Efi Merdler

    Red Orchid Guest

    Thomas Hawtin <> wrote or quoted in
    Message-ID: <45721e2f$0$8748$>:

    >
    > try {
    > Writer rawOut = ...l;
    > try {
    > BufferedWriter out = new BufferedWriter(rawOut);
    > ...
    > out.flush();
    > } finally {
    > rawOut.close();
    > }
    > } catch (IOException exc) {
    > ...
    > }
    >




    What is your comment about the following codes ?

    <code_0>
    try {
    try {
    throw new Exception("E_0"); // Exception_0
    }
    finally {
    int i = 5;
    i /= 0; // Exception_1
    }
    }
    catch (Exception e) {
    e.printStackTrace();
    }
    </code_0>

    The "code_0" above prints only "Exception_1". That is,
    "Exception_0" is not printStackTraced.

    Next,
    <code_1>
    try {
    throw new Exception("E_0"); // Exception_0
    }
    catch (Exception e) {
    e.printStackTrace();
    }
    finally {
    try {
    int i = 5;
    i /= 0; // Exception_1
    }
    catch (Exception e) {
    e.printStackTrace();
    }
    }
    </code_1>

    The "code_1" above prints both "Exception_0" and "Exception_1".

    Last,
    <code_2>
    void processXX(...) throws IOException {
    ...
    try {
    ... stream = .....;
    // some IOException(*) occurs.
    ..
    }
    finally {
    stream.close();
    }
    }
    </code_2>

    In "code_2" above, if IOException of "stream.close()" occurs,
    "IOException(*)" is discarded.

    I think that it is better to isolate IOException of "stream.close()"
    from other IOExceptions. For example,

    <code_3>
    void processXX(...) throws IOException, ... {
    ...
    try {
    ... stream = .....;
    // some IOException(*) occurs.
    ..
    }
    finally {
    try {
    stream.close();
    }
    catch (IOException e) {
    // process or throw new IOCloseException(...);
    }
    }
    }
    ....
    class IOCloseException extends Exception {
    ....
    }
    </code_3>
    Red Orchid, Dec 3, 2006
    #4
  5. Efi Merdler

    Chris Smith Guest

    Red Orchid <> wrote:
    > What is your comment about the following codes ?
    >


    Here's my comment. What you get is fairly simple code that reports a
    problem. Once you fix that problem, another problem becomes visible.
    This works fine. It may initially seem less than ideal, but upon
    further examination, there's not really much of a better option. There
    are a few reasons for this.

    First, it is impossible for a method to reasonably respond to two
    different exceptions to its caller. It can't throw both of them. In
    order to handle both exceptions, you would need to do so within the
    current method. That implies that you scatter code to report unexpected
    exceptions all over your code base, which is very ugly. That's what
    your code does, for example; it calles e.printStackTrace to print the
    exception to standard error; which is, of course, not what you want to
    do 95% of the time. Most of the time, you want to write the exception
    to a ServletContext log, or email it to an administrator, or pass it to
    log4j; but which one depends on the context where the code is used.

    Second, it just doesn't matter, in practice, whether you get both
    exceptions or not. That's because the second exception generally
    reports exactly the same error condition as the first. (That's really
    obvious in your I/O stream example; why would you want two different
    stack traces representing the same problem with a file?) Even if they
    were different, multiple simultaneous failures is hardly the kind of
    thing that's worth introducing extra complexity into your code just to
    handle marginally better.

    > I think that it is better to isolate IOException of "stream.close()"
    > from other IOExceptions. For example,


    I'm really curious here. Do you really think thatg closing the stream
    is failing for a different reason than the original I/O operation? Is
    there even one plausible scenario where that's true (that close() would
    have failed, except that something unrelated has gone wrong?)

    --
    Chris Smith
    Chris Smith, Dec 3, 2006
    #5
  6. Efi Merdler

    Red Orchid Guest

    Chris Smith <> wrote or quoted in
    Message-ID: <>:

    > Here's my comment. What you get is fairly simple code that reports a
    > [snip]
    > handle marginally better.
    >



    My previous article has a connection with Tom Hawtin's article.
    I do not want to deduce your comment on his example from
    your comment on my previous article. What is your comment
    on Tom Hawtin's example ?

    There is my additional view of Tom Hawtin's example. First,
    the following code is a part of "BufferedWriter" source.

    <code>
    public void close() throws IOException {
    synchronized (lock) {
    if (out == null)
    return;
    flushBuffer();
    out.close();
    out = null; // #1.
    cb = null; //
    }
    }
    </code>

    I think that the author of "BufferedWriter" has intention to
    assign null to "out" and "cb" when "close()" is called.

    But, the following code discards the intention because
    "out.close()" is not called.

    <code>
    //
    // quoted from Tom Hawtin's article.
    //
    try {
    Writer rawOut = ...l;
    try {
    BufferedWriter out = new BufferedWriter(rawOut);
    ...
    out.flush();
    } finally {
    rawOut.close();
    }
    } catch (IOException exc) {
    ...
    }
    <code>



    > I'm really curious here. Do you really think thatg closing the stream
    > is failing for a different reason than the original I/O operation? Is
    > there even one plausible scenario where that's true (that close() would
    > have failed, except that something unrelated has gone wrong?)
    >




    I think that IOException of steam should not make a mess in
    consistency of performance. Lets consider the following code.

    <code_2>
    //
    // quoted and modified from my previous article.
    //
    void processXX(T1 o1, T2 o2) throws XXException, IOException {

    ... stream = ...;
    try {
    { code block #1} // The state of o1, o2 are changed;
    { code block #2} // data are IOed to/from stream.
    ....
    }
    finally {
    stream.close();
    }
    }

    //
    // Main routine..
    //
    try {
    ...
    processXX(o1, o2);
    ...
    }
    catch (XXException e) {
    // restore o1, o2.
    }
    catch (IOException e) {
    // ..
    }
    </code_2>


    The method "processXX" do not guarantee the consistency
    of performance.

    After { code block #1}, if XXException occurs and successively
    IOException of "stream.close()" occurs, the state of o1, o2 can
    not be restored to the previous state. Because the XXException
    was discarded by the IOException. ( Consider more than one
    XXExceptions.)

    Closing the stream will be not failing for a different reason than
    the original I/O operation. But, IOException of stream.close()
    can swallow up other exceptions (XXExceptions).

    I think, the failure of "stream.close()" makes no matter in practice.
    It is important that other exceptions can be swallowed up.

    Do you think that the following code implies that a developer scatter
    code to report unexpected exceptions all over his code base ?

    <code>
    try {
    ...
    }
    catch (...) {
    ...
    }
    finally {
    try {
    stream.close();
    }
    catch (Exception e) {
    ...
    }
    }
    </code>
    Red Orchid, Dec 3, 2006
    #6
  7. Red Orchid wrote:
    > What is your comment about the following codes ?
    > try {
    > try {
    > throw new Exception("E_0"); // Exception_0
    > }
    > finally {
    > int i = 5;
    > i /= 0; // Exception_1
    > }
    > }
    > catch (Exception e) {
    > e.printStackTrace();
    > }


    My first comment is that printStackTrace is not an acceptable exception
    handling problem (it really should have "debug" in its name).

    My second comment is that the exception from finally should be at least
    as significant as the first exception. There shouldn't be much happening
    in the finally block. Now, you've switched to a made up example, but the
    original, writing to a stream, is common. In that case, if you can't
    even close the file you have real problems - at least as bad as not
    being able to write to it.

    > The "code_1" above prints both "Exception_0" and "Exception_1".


    So is that going to be two dialog boxes with cryptic and entirely
    unhelpful messages? ;)

    Tom Hawtin
    Thomas Hawtin, Dec 3, 2006
    #7
  8. Red Orchid wrote:
    > There is my additional view of Tom Hawtin's example. First,
    > the following code is a part of "BufferedWriter" source.


    > public void close() throws IOException {
    > synchronized (lock) {
    > if (out == null)
    > return;
    > flushBuffer();
    > out.close();
    > out = null; // #1.
    > cb = null; //
    > }
    > }


    > I think that the author of "BufferedWriter" has intention to
    > assign null to "out" and "cb" when "close()" is called.


    Look at the code. If the flush throws an exception, the out is not
    closed. In fact you can't close out at all through BufferedWriter if you
    it is failing on the flush. For 1.5 and earlier you need to flush the
    buffer and call close on the underlying stream. Checking the bug database:

    http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6266377

    (Something similar exists, and also fixed in 1.6, for BufferedOutputStream.)

    > But, the following code discards the intention because
    > "out.close()" is not called.


    Who cares. We flushed the buffer. The BufferedWriter is done. All
    BufferedWriter.close does extra is to make sure close closes it from
    further method calls (which we are not going to do).


    > The method "processXX" do not guarantee the consistency
    > of performance.


    Then it's badly designed. True exception safety comes through accepting
    failure may occur at any point. So it's useless trying to write code for
    each failure. Keep it simple.

    Tom Hawtin
    Thomas Hawtin, Dec 3, 2006
    #8
  9. Efi Merdler

    Red Orchid Guest

    Thomas Hawtin <> wrote or quoted in
    Message-ID: <4572c7eb$0$8758$>:

    > [snip]
    > My second comment is that the exception from finally should be at least
    > as significant as the first exception. There shouldn't be much happening
    > in the finally block. Now, you've switched to a made up example, but the
    > original, writing to a stream, is common. In that case, if you can't
    > even close the file you have real problems - at least as bad as not
    > being able to write to it.
    >




    The point of my previous article is that an exception of "finally"
    block can discard other exceptions that have to be treated.


    This is a part of OP's code. Note that #1 was not defined.
    <quote>
    try {
    .... // code_block: #1
    } catch (IOException e) {
    ....
    }
    finally {
    if (out != null)
    out.close();
    }
    </quote>


    This is a part of your previous article.
    <quote>
    try {
    Writer rawOut = ...l;
    try {
    BufferedWriter out = new BufferedWriter(rawOut);
    ... // code_block: #2
    out.flush();
    } finally {
    rawOut.close();
    }
    } catch (IOException exc) {
    ... // code_block: #3
    }
    </quote>

    In #2, it is practicable for an instance(*) of some class to throw
    a sub-class exceptions(*) of IOException.
    If "rawOut.close()" throws "IOException", the exceptions(*) can
    not be treated in #3 although they have to be treated, because
    they were swallowed up by the IOException of "rawOut.close()".



    >
    > So is that going to be two dialog boxes with cryptic and entirely
    > unhelpful messages? ;)
    >


    Treating the exceptions(*) includes the action of restoring
    the instance(*) to the previous state, besides dialog box.

    That is,
    <code>
    Writer rawOut = ...;
    try {
    BufferedWriter out = new BufferedWriter(rawOut);
    ...
    {code block #4} // XxxxIOException can be thrown.
    ...
    out.close();
    }
    catch (XxxxIOException e) {
    // Restore a related instance of some class to the previous
    // state. Or do something that is required.
    }
    catch (IOException e) {
    // dialog box or do something.
    }
    finally {
    try {
    rawOut.close();
    }
    catch (IOException e) {
    // process
    }
    }
    </code>


    Of course, in #3 of your code, you can roll all the related
    instances back. But, to do such policy is to hide problems
    that happened. If the policy is valid, why do Java have so
    many kinds of exceptions ?
    Red Orchid, Dec 3, 2006
    #9
  10. Efi Merdler

    Chris Smith Guest

    Red Orchid <> wrote:
    > My previous article has a connection with Tom Hawtin's article.


    Yes, I know. More generally, though, Thomas Hawtin suggested nesting a
    try/finally inside of a try/catch. That's a very wise suggestion, and
    one that I agree with. You objected, and I answered your objections.
    If you want to have a private conversation with Thomas, then email is a
    good medium for that.

    > I do not want to deduce your comment on his example from
    > your comment on my previous article. What is your comment
    > on Tom Hawtin's example ?


    I think the strategy is sound.

    > I think that the author of "BufferedWriter" has intention to
    > assign null to "out" and "cb" when "close()" is called.


    Why would it matter? The instance of BufferedWriter is already gone by
    the time it might make any difference. The spec for BufferedWriter is
    sufficient to guarantee that calling flush() and then discarding the
    instance is a perfectly sane thing to do. That's all rather incidental,
    though, to the point that try/finally should be separated from try/catch
    blocks.

    > try {
    > ...
    > processXX(o1, o2);
    > ...
    > }
    > catch (XXException e) {
    > // restore o1, o2.
    > }
    > catch (IOException e) {
    > // ..
    > }


    Yes, that's a problem? What is your alternative? Do you intend to
    throw both XXException and IOException from processXX? The language
    doesn't allow that. Do you intend to swallow the IOException and just
    throw XXException? That's really no better.

    My alternative is that if XXException is supposed to be a recoverable
    exception, then it should leave the state of o1 and o2 as they were
    before throwing XXException. Preferably, this is done by modifying the
    logic to not modify them in the first place. If necessary, though, it
    can be done by adding another try/catch block in processXX, restoring o1
    and o2, and then rethrowing the exception. (Another solution, probably
    even better when it is reasonably in the first place, is to eliminate
    side-effects from processXX in the first place.)

    > Do you think that the following code implies that a developer scatter
    > code to report unexpected exceptions all over his code base ?


    > finally {
    > try {
    > stream.close();
    > }
    > catch (Exception e) {
    > ...
    > }
    > }


    Yes, I do. Otherwise, you would have specified what goes in the catch
    block instead of using an ellipse. The problem is that you don't know
    what ought to go in the catch block. It depends on how the API is being
    used.

    --
    Chris Smith
    Chris Smith, Dec 3, 2006
    #10
  11. Red Orchid wrote:
    > Thomas Hawtin <> wrote or quoted in
    > Message-ID: <4572c7eb$0$8758$>:
    >
    >> [snip]
    >> My second comment is that the exception from finally should be at least
    >> as significant as the first exception. There shouldn't be much happening
    >> in the finally block. Now, you've switched to a made up example, but the
    >> original, writing to a stream, is common. In that case, if you can't
    >> even close the file you have real problems - at least as bad as not
    >> being able to write to it.

    >
    > The point of my previous article is that an exception of "finally"
    > block can discard other exceptions that have to be treated.


    As I say, in any sane system, those other exceptions have become
    obsoleted by the finally exception.

    > In #2, it is practicable for an instance(*) of some class to throw
    > a sub-class exceptions(*) of IOException.
    > If "rawOut.close()" throws "IOException", the exceptions(*) can
    > not be treated in #3 although they have to be treated, because
    > they were swallowed up by the IOException of "rawOut.close()".


    Yup, and it's still obsolete.

    > Treating the exceptions(*) includes the action of restoring
    > the instance(*) to the previous state, besides dialog box.


    Such clean-up actions are required no matter what the exception. So,
    they should be in a finally clause, not in a catch clause. It's
    generally much easier to work on a copy rather than attempting some roll
    back procedure, so such restoring actions are unnecessary.

    > catch (XxxxIOException e) {
    > // Restore a related instance of some class to the previous
    > // state. Or do something that is required.
    > }


    You need this whatever the exception. i.e. it must be in a finally (or
    potentially catch Throwable) block (possibly with a boolean to check
    whether try block executed successfully).

    > catch (IOException e) {
    > // dialog box or do something.
    > }
    > finally {
    > try {
    > rawOut.close();
    > }
    > catch (IOException e) {
    > // process


    Should be the same problem as the previous, or more urgent.

    > Of course, in #3 of your code, you can roll all the related
    > instances back. But, to do such policy is to hide problems
    > that happened. If the policy is valid, why do Java have so
    > many kinds of exceptions ?


    There are different exceptions so that you can diagnose the problem
    (possibly). You are going to have fun trying to disentangle multiple
    exceptions at once. The most important exception should be the outermost
    one.

    Tom Hawtin
    Thomas Hawtin, Dec 3, 2006
    #11
  12. Efi Merdler

    Red Orchid Guest

    Thomas Hawtin <> wrote or quoted in
    Message-ID: <457306ae$0$8759$>:

    > There are different exceptions so that you can diagnose the problem
    > (possibly). You are going to have fun trying to disentangle multiple
    > exceptions at once. The most important exception should be the outermost
    > one.
    >



    I have read your comments.

    And,
    Do the word "outermost" above mean this ?

    <code>
    try {
    ...
    }
    catch (XException e) {
    ...
    }
    catch (YException e) { // <- Outermost
    ...
    }

    Or,

    try {
    try {
    ...
    }
    catch (XException e) {
    ...
    }
    }
    catch (YException e) { // <- Outermost
    ...
    }
    </code>


    If XException is super class of YException,
    YException is unreachable.
    Red Orchid, Dec 4, 2006
    #12
  13. Efi Merdler

    Red Orchid Guest

    Chris Smith <> wrote or quoted in
    Message-ID: <>:

    >
    > [snip]
    >
    > > finally {
    > > try {
    > > stream.close();
    > > }
    > > catch (Exception e) {
    > > ...
    > > }
    > > }

    >
    > Yes, I do. Otherwise, you would have specified what goes in the catch
    > block instead of using an ellipse. The problem is that you don't know
    > what ought to go in the catch block. It depends on how the API is being
    > used.
    >



    At any rate, it is possible that all the close handling of streams
    related with reading/writing data are processed with only one method
    in the entire scope of your project. For instance, inside all the "finally"
    blocks of your project:
    <code>
    ....
    finally {
    YourProjectUtility.closeFileStream(stream, file);
    }

    //
    // YourProjectUtility
    //
    public static void closeFileStream(AnyStreamType stream, File dataFile) {
    if (stream != null) {
    try {
    stream.close();
    }
    catch (IOException e) {
    // Maybe, DialogBox( ..... , dataFile, e.getMessage(), ..);
    }
    }
    }
    </code>
    Red Orchid, Dec 4, 2006
    #13
  14. Efi Merdler

    Chris Smith Guest

    Red Orchid <> wrote:
    > At any rate, it is possible that all the close handling of streams
    > related with reading/writing data are processed with only one method
    > in the entire scope of your project. For instance, inside all the "finally"
    > blocks of your project:


    > finally {
    > YourProjectUtility.closeFileStream(stream, file);
    > }


    That's nice, if all the code that I write is only used within one
    application. If, on the other hand, I want to learn from half a century
    of software development experience and build modular code that can be
    used in a number of different places, then this is a bad idea.

    --
    Chris Smith
    Chris Smith, Dec 4, 2006
    #14
  15. Efi Merdler

    Red Orchid Guest

    Chris Smith <> wrote or quoted in
    Message-ID: <>:

    >
    > > finally {
    > > YourProjectUtility.closeFileStream(stream, file);
    > > }

    >
    > That's nice, if all the code that I write is only used within one
    > application. If, on the other hand, I want to learn from half a century
    > of software development experience and build modular code that can be
    > used in a number of different places, then this is a bad idea.
    >



    Whether this is a bad idea or not will be depend upon the type of a project.
    And, the maintenance related with the above code will not be hard at least
    (Including remove and reuse).
    Red Orchid, Dec 4, 2006
    #15
  16. Red Orchid wrote:
    > Thomas Hawtin <> wrote or quoted in
    > Message-ID: <457306ae$0$8759$>:
    >
    >> There are different exceptions so that you can diagnose the problem
    >> (possibly). You are going to have fun trying to disentangle multiple
    >> exceptions at once. The most important exception should be the outermost
    >> one.


    > Do the word "outermost" above mean this ?


    By outermost, I mean the one thrown last (from an execution point of
    view). There is no retry mechanism in Java (they do not appear to work
    well in practice), so exceptions are always thrown outwards.

    It's trivial on the acquire side of things, as the code causing the
    innermost exception doesn't get to be executed.

    > try {
    > ...
    > }
    > catch (XException e) {
    > ...
    > }
    > catch (YException e) { // <- Outermost
    > ...
    > }


    Here both catch blocks would be equally outermost. An exception thrown
    from the first catch would not be caught by the second.

    > try {
    > try {
    > ...
    > }
    > catch (XException e) {
    > ...
    > }
    > }
    > catch (YException e) { // <- Outermost
    > ...
    > }


    Here if the inner catch block threw a YException, then the YException
    would be outermost.

    > If XException is super class of YException,
    > YException is unreachable.


    And your code wouldn't compile.

    Tom Hawtin
    Thomas Hawtin, Dec 4, 2006
    #16
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Matt
    Replies:
    4
    Views:
    709
    Adam Maass
    Jul 1, 2004
  2. Matt
    Replies:
    6
    Views:
    724
  3. Steve Claflin
    Replies:
    2
    Views:
    378
    Rhino
    Feb 13, 2006
  4. morrell
    Replies:
    1
    Views:
    926
    roy axenov
    Oct 10, 2006
  5. David Lozzi

    Try...Catch...Finally not firing finally?

    David Lozzi, Apr 23, 2007, in forum: ASP .Net
    Replies:
    12
    Views:
    777
    Alvin Bruney [MVP]
    May 11, 2007
Loading...

Share This Page