 |
 | Home | Contact Us | FAQ | Search & Site Map | Link to Us |
 | Sign In | Join | Other 45 Sites in Network |
Java Forum / General / December 2006Exception in finally block
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, > Assume I have [quoted text clipped - 20 lines] > Thanks, > Efi Hello, try use try-catch block in finally block For Example: try { ... } catch (Exception e) { ... } finally { try { ... } catch (Exception e) { ... } } Damian Dunajski > Hello, > Assume I have [quoted text clipped - 17 lines] > 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 <usenet@tackline.plus.com> wrote or quoted in Message-ID: <45721e2f$0$8748$ed2619ec@ptn-nntp-reader02.plus.net>: > try { > Writer rawOut = ...l; [quoted text clipped - 8 lines] > ... > } 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> > 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?)  SignatureChris Smith Chris Smith <cdsmith@twu.net> wrote or quoted in Message-ID: <MPG.1fdc23c27eb9a34989776@news.altopia.net>: > 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> > 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) { [quoted text clipped - 6 lines] > } > } > 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 > 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 { > ... [quoted text clipped - 7 lines] > // .. > } 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 { [quoted text clipped - 4 lines] > } > } 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. SignatureChris Smith Chris Smith <cdsmith@twu.net> wrote or quoted in Message-ID: <MPG.1fdcadde86fd2b36989778@news.altopia.net>: > [snip] > [quoted text clipped - 11 lines] > 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> > 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. SignatureChris Smith Chris Smith <cdsmith@twu.net> wrote or quoted in Message-ID: <MPG.1fdd4b997706833098977b@news.altopia.net>: > > finally { > > YourProjectUtility.closeFileStream(stream, file); [quoted text clipped - 4 lines] > 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). > What is your comment about the following codes ? > try { [quoted text clipped - 9 lines] > 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 <usenet@tackline.plus.com> wrote or quoted in Message-ID: <4572c7eb$0$8758$ed2619ec@ptn-nntp-reader02.plus.net>: > [snip] > My second comment is that the exception from finally should be at least [quoted text clipped - 3 lines] > 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 ? > Thomas Hawtin <usenet@tackline.plus.com> wrote or quoted in > Message-ID: <4572c7eb$0$8758$ed2619ec@ptn-nntp-reader02.plus.net>: [quoted text clipped - 9 lines] > 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. [quoted text clipped - 5 lines] > 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 <usenet@tackline.plus.com> wrote or quoted in Message-ID: <457306ae$0$8759$ed2619ec@ptn-nntp-reader02.plus.net>: > 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. > Thomas Hawtin <usenet@tackline.plus.com> wrote or quoted in > Message-ID: <457306ae$0$8759$ed2619ec@ptn-nntp-reader02.plus.net>: [quoted text clipped - 3 lines] >> 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 { > ... [quoted text clipped - 5 lines] > ... > } Here both catch blocks would be equally outermost. An exception thrown from the first catch would not be caught by the second. > try { > try { [quoted text clipped - 7 lines] > ... > } 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 Free MagazinesGet these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...
|
Reply to this Message
|
 Sign In
 Join
 My Latest Posts My Monitored Threads My Blog My Photo Gallery My Profile My Homepage Start New Thread Enable EMail Alerts Rate this Thread
|
©2008 Advenet LLC Privacy Policy - Terms of Use
This website includes both content owned or controlled by Advenet as well as content owned or controlled by third parties.