Skip to main content
GameDev.net gamedev.net
🔒 Locked

Should Dispose throw?

Started by Telastyn Aug 25, 2010 at 10:40 AM 9 replies 2.7k views
Original Post
Telastyn
Telastyn
This came up in another thread, but I think it perhaps best to split off into its own discussion. I personally think that it's quite unfortunate that .NET library classes can throw exceptions as part of their Dispose method. None of the examples I can think of provide any useful data other than something didn't close cleanly. I also think that it should pretty much be avoided as a best practice.

So, are there good examples of disposable objects that have dispose failure conditions that are recoverable? A buffered stream was offered in the other thread (if the buffer can't be flushed), though that's not particularly compelling.

Or am I just carrying over the 'don't throw from destructors' best practice from C++ too far?
Antheus
Antheus
Dispose/finalizer/etc are not destructors. Objects as such cannot leak, so there is no reason for them not to throw.

In another post I said that when working in Java memory and other resources are unlimited. This carries over to C#, so if your application is absolutely critical or absurdly constrained (such as 500 redundant open sockets causing problems) then either abandon the offending classes in favor of fully deterministic cleanup, or tackle each case individually.

Otherwise, buy more RAM or disk or something. Experience shows that generic solution to cleanup isn't needed so trouble spots are best tackled on case-per-case basis.

Quote:
So, are there good examples of disposable objects that have dispose failure conditions that are recoverable?
Yes - C.

C libraries have seen decades of real world use in some of the most constrained environments. To learn good resource management practices, understand why the syscalls work the way they do. It might be obvious that there are no fancy streams and that entire internet is built on top of 4 functions.

Managed memory and other conveniences are optimization and commodization of programmers - not improvement over development techniques. The problem here is - managed languages manage memory - but that is just one of resources applications use - and an effectively irrelevant one these days. Even worse, file systems and other external states are not under exclusive control, which makes GC-like approach impossible.

Simply put - C# does absolutely nothing for resources other than memory, so whatever options you choose, all the lessons from other languages apply. They also explain why C++ is so mindbogglingly complex.
rip-off
rip-off
Quote:

None of the examples I can think of provide any useful data other than something didn't close cleanly.

Well, it is nice to be able to report to the user that their file might not have been saved (or whatever). Dispose could have been specified to return a boolean indicating if the object was disposed without errors, but then you get the classic case of return values being silently ignored. Plus that wouldn't work well with "using" statements.

It is a hard call. Its certainly annoying to get additional exceptions when you are trying to clean up after one. In some code you are lucky if it was written to correctly handle the first exception, let alone some cascade of them.

Quote:

So, are there good examples of disposable objects that have dispose failure conditions that are recoverable?

Locally, I don't think so. But I think the theory is that the calling code might have a way (unlikely as this seems). Perhaps during writing the file to the hard disk, maybe the operation could be retried on a different disk. You are mostly talking about re-starting from scratch though. Still, better than crashing and losing your work.
blog | twitter
sam_hughes
sam_hughes
If Dispose methods didn't throw, I'm pretty sure my previous job would be a nightmare. Even if an error can't be recovered from (at least not right away), it can (must) be reported or logged. And then usually the operation would be retried later.

It's a very, very easy call to make, from my experience. I've spent a lot of time at my previous job handling errors reported on clients where things threw exceptions and stack traces were dumped in log files, and I've never thought, "Oh, if only this Dispose call hadn't thrown an exception, blocking me from seeing that other exception's value!"

Part of the reason is this: If the exceptions are dependent on one another, you're going to have at least some useful information about the underlying problem from the Dispose exception. If the exceptions are not dependent on one another, that's a very rare situation, and the exception thrown in a finally block is still a bug that needs to be fixed, and when that's fixed, you'll notice the other error, if it's still happening. The importance of noticing the error is far greater than the downside of having a non-ideal stack trace for a subset of errors.

The solution (in the language design sense) to this problem of exceptions-hiding-exceptions is found in D: exceptions form a linked list, and when you throw an exception from within a finally block, the currently-thrown exception, if there is one, gets chained to the newly thrown one.

And, you know, I have no idea in what sense silently failing could possibly be considered good practice. I have no clue where you're coming from.
MaulingMonkey
MaulingMonkey
Not throwing an exception doesn't mean you can't still log an error -- it just means you don't abuse an execution flow construct to do it. Typically, you would cut out the middle man and be more direct about it.

Granted, C#'s practice of switching to the second exception instead of calling std::terminate like C++ does (which isn't exactly 'oh I want to log some exceptions' friendly) makes throwing from Dispose() less absolutely horribly cringe-worthy.



Quote:
Or am I just carrying over the 'don't throw from destructors' best practice from C++ too far?

"Iunnolol."

I'm generally inclined to not directly throw from Dispose myself -- even if it works, hiding the first exception isn't exactly pretty when there's likely a cleaner approach -- but I haven't gone out of my way to ensure it couldn't possibly throw either, and so far, I haven't been bitten by it.

On the subject, the habit of WinForms eating Paint event exceptions does make me rage.
Telastyn
Telastyn
Quote:
Original post by rip-off
Perhaps during writing the file to the hard disk, maybe the operation could be retried on a different disk.


Wouldn't that fail though on open or write, not dispose? Or were you talking about the helpfully flushing buffered stream example?

Quote:

I've spent a lot of time at my previous job handling errors reported on clients where things threw exceptions and stack traces were dumped in log files, and I've never thought, "Oh, if only this Dispose call hadn't thrown an exception, blocking me from seeing that other exception's value!"


That's odd. I've spent plenty of time hunting through exception logs and wished that all this 'connection already faulted' garbage wasn't obscuring the actual error (or filling up the logs, which led to more errors).

Quote:

And, you know, I have no idea in what sense silently failing could possibly be considered good practice. I have no clue where you're coming from.


I'm saying that for every (non-exploitive) scenario I can think of, Dispose exceptions have 1 sane path for handling gracefully: catch, then either drop it or hard-terminate the resource. I find it to be... not very C#-like to require that boilerplate code or else run the risk of crashing your app on something like an unclean close (ignoring perhaps the fact that the resource acquisition part of using blocks are almost guaranteed to throw, meaning they're getting wrapped in try/catch anyways).
Antheus
Antheus
Quote:
Original post by Telastyn

I'm saying that for every (non-exploitive) scenario I can think of, Dispose exceptions have 1 sane path for handling gracefully: catch, then either drop it or hard-terminate the resource.

Say socket.close() throws? What do you do? Has the resource been cleaned up? Or not? Is it petting Schrodinger's cat? Or is close() merely a hint to beat GC?

Same for files and other external resources. Not having access to fsync, what does file.close() failing mean? Was buffer flushed? Was descriptor released? Or stream failing - was the underlying resource flushed?

Because after 10 years, I don't know how to properly handle those cases. For sockets or files I just assume that final manual flush() succeeding means we're done, and then I ignore anything beyond that. It's not like I can do anything useful.


By extension, if something important is lost because of thrown exception, the problem lies in error handling logic. Is "failure to completely write to file" problem with disposable concept or with application not ensuring it? And by extension, "robust" code will never need to throw in dispose, since by the time resource is ready for disposal, everything else has been properly finished.

Quote:
are there good examples of disposable objects that have dispose failure conditions that are recoverable
Nope. If first call fails with exception - subsequent invocations must do nothing as per requirement.

It's the limbo problem of failed C++ constructors. How to handle partially constructed object. Consequences are the same, object is left in undetermined state. In C# it cannot break, but whatever class invariants may have existed can no longer be properly enforced.

Fortunately, this type of problems tends to occur fairly rarely in practice, at least as far as some generally correct solution is concerned. Often just fixing whatever stack trace was dumped is more than enough, if even necessary.
sam_hughes
sam_hughes
The unhappy fact is that even though we don't like it, Dispose methods will sometimes need to report exceptions.

Quote:
I find it to be... not very C#-like to require that boilerplate code or else run the risk of crashing your app on something like an unclean close (ignoring perhaps the fact that the resource acquisition part of using blocks are almost guaranteed to throw, meaning they're getting wrapped in try/catch anyways).


Now this is very peculiar. You put try/catch blocks around all your using blocks? And handle every exception in a fine-grained manner? Using blocks were not designed for this.
Telastyn
Telastyn
Quote:
Original post by sam_hughes
Now this is very peculiar. You put try/catch blocks around all your using blocks? And handle every exception in a fine-grained manner? Using blocks were not designed for this.


Not every one. Sorry if I implied that. If the local code needs to handle resource acquisition exceptions (which isn't terribly infrequent) there will be though.

And if Dispose can throw, I generally will handle that locally too so to not force the calling code to deal with something since it's usually irrelevant. Hence the thread...
Ibasa
Ibasa
Keep finalizers in mind as often they are what calls dispose. And if a finalizer throws an exception at application exit then all pending finalizers will not run. (N.B. oldnewthing's CLR week 2010 was about the GC and finalizers and is worth a look)
MaulingMonkey
MaulingMonkey
Quote:
Original post by Ibasa
Keep finalizers in mind as often they are what calls dispose. And if a finalizer throws an exception at application exit then all pending finalizers will not run. (N.B. oldnewthing's CLR week 2010 was about the GC and finalizers and is worth a look)

Interestingly, in .NET 1.1 it would simply eat the exceptions. The documentation implies that in 2.0 it changed to some sort of policy which defaulted to killing the process outright, even if the finalizers were simply being called mid-process. This program never reaches the last line of Main:

using System;namespace ConsoleApplication1 {	class Foo {		~Foo() { Console.WriteLine("Oh noes"); throw new Exception("!"); }	}	class Program {		static void Main( string[] args ) {			Foo f = new Foo();			f = null;			GC.Collect();			GC.WaitForPendingFinalizers();			Console.WriteLine("Guess it didn't terminate.");		}	}}
Oh noesUnhandled Exception: System.Exception: !   at ConsoleApplication1.Foo.Finalize() in I:\home\projects\new\ConsoleApplication1\ConsoleApplication1\Program.cs:line 5Press any key to continue . . . <font color="grey">_</font>


Definitely something to keep in mind, even if Finalizers are a tool that should be used extremely sparingly in the first place.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.