StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

PO
text
Body
copied!<p><strong>Update:</strong> My original response didn't really answer the question so here is another attempt...</p> <p>To help solve the root problem, developers forgetting to throw <code>ObjectDisposedExceptions</code>, perhaps automated unit testing will do the trick. If you want to be strict about requiring all methods/properties to throw <code>ObjectDisposedException</code> immediately if <code>Dispose</code> has already been called then you can use the following unit test. Just specify the assemblies you want to test. You will probably need to modify the <code>IsExcluded</code> method as necessary and the object mocking may not work in all cases.</p> <pre class="lang-cs prettyprint-override"><code>using System; using System.Collections.Generic; using System.Linq; using System.Reflection; using MbUnit.Framework; using Moq; [TestFixture] public class IDisposableTests { [Test] public void ThrowsObjectDisposedExceptions() { var assemblyToTest = Assembly.LoadWithPartialName("MyAssembly"); // Get all types that implement IDisposable var disposableTypes = from type in assemblyToTest.GetTypes() where type.GetInterface(typeof(IDisposable).FullName) != null select type; foreach (var type in disposableTypes) { // Try to get default constructor first... var constructor = type.GetConstructor(Type.EmptyTypes); if (constructor == null) { // Otherwise get first parameter based constructor... var constructors = type.GetConstructors(); if (constructors != null && constructors.Length > 0) { constructor = constructors[0]; } } // If there is a public constructor... if (constructor != null) { object instance = Activator.CreateInstance(type, GetDefaultArguments(constructor)); (instance as IDisposable).Dispose(); foreach (var method in type.GetMethods()) { if (!this.IsExcluded(method)) { bool threwObjectDisposedException = false; try { method.Invoke(instance, GetDefaultArguments(method)); } catch (TargetInvocationException ex) { if (ex.InnerException.GetType() == typeof(ObjectDisposedException)) { threwObjectDisposedException = true; } } Assert.IsTrue(threwObjectDisposedException); } } } } } private bool IsExcluded(MethodInfo method) { // May want to include ToString, GetHashCode. // Doesn't handle checking overloads which would take more // logic to compare parameters etc. if (method.Name == "Dispose" || method.Name == "GetType") { return true; } return false; } private object[] GetDefaultArguments(MethodBase method) { var arguments = new List<object>(); foreach (var parameter in method.GetParameters()) { var type = parameter.ParameterType; if (type.IsValueType) { arguments.Add(Activator.CreateInstance(type)); } else if (!type.IsSealed) { dynamic mock = Activator.CreateInstance(typeof(Mock<>).MakeGenericType(type)); arguments.Add(mock.Object); } else { arguments.Add(null); } } return arguments.ToArray(); } } </code></pre> <hr> <p><strong>Original Response</strong>: It doesn't look like there is anything like NotifyPropertyWeaver for <code>IDisposable</code> so if you want that you'll need to create a similar project yourself. You can potentially save yourself a little work by having a base Disposable class like in this <a href="http://davybrion.com/blog/2008/06/disposing-of-the-idisposable-implementation/" rel="nofollow">blog entry</a>. Then you just have a one-liner at the top of each method: <code>ThrowExceptionIfDisposed()</code>. </p> <p>However, neither possible solution feel right or seem necessary. In general throwing <code>ObjectDisposedException</code> is not needed that often. I did a quick search in Reflector and the <code>ObjectDisposedException</code> is thrown directly by just 6 types in the BCL, and for a sample outside the BCL <code>System.Windows.Forms</code> only has one type that throws: <code>Cursor</code> when getting the Handle. </p> <p>Basically, you only need to throw <code>ObjectDisposedException</code> if a call on your object will fail specifically because <code>Dispose</code> was already called, such as if you set some field to null that is needed by a method or property. An <code>ObjectDisposedException</code> will be more informative than a random <code>NullReferenceException</code>, but unless you're cleaning up unmanaged resources it's not usually necessary to set a bunch of fields to null. Most of the time if you're just calling Dispose on other objects, it's up to them to throw <code>ObjectDisposedException</code>.</p> <p>Here is a trivial example you might throw <code>ObjectDisposedException</code> explicitly:</p> <pre class="lang-cs prettyprint-override"><code>public class ThrowObjectDisposedExplicity : IDisposable { private MemoryStream stream; public ThrowObjectDisposedExplicity() { this.stream = new MemoryStream(); } public void DoSomething() { if (this.stream == null) { throw new ObjectDisposedException(null); } this.stream.ReadByte(); } protected virtual void Dispose(bool disposing) { if (disposing) { if (this.stream != null) { this.stream.Dispose(); this.stream = null; } } } public void Dispose() { this.Dispose(true); GC.SuppressFinalize(this); } } </code></pre> <p>With the code above though there is really no need set stream to null. You could just rely on the fact that <code>MemoryStream.ReadByte()</code> will throw <code>ObjectDisposedException</code> on it's own like the code below:</p> <pre class="lang-cs prettyprint-override"><code>public class ThrowObjectDisposedImplicitly : IDisposable { private MemoryStream stream; public ThrowObjectDisposedImplicitly() { this.stream = new MemoryStream(); } public void DoSomething() { // This will throw ObjectDisposedException as necessary this.stream.ReadByte(); } protected virtual void Dispose(bool disposing) { if (disposing) { this.stream.Dispose(); } } public void Dispose() { this.Dispose(true); GC.SuppressFinalize(this); } } </code></pre> <p>The first strategy of setting stream to null may make sense in some cases, such as if you know the object will throw an exception if Dispose is called multiple times. In that case you want to be defensive and make sure your class doesn't throw an exception on multiple calls to <code>Dispose</code>. Besides that I can't think of any other cases off hand where you would need to set fields to null which would probably require throwing <code>ObjectDisposedExceptions</code>.</p> <p>Throwing an <code>ObjectDisposedException</code> isn't needed often and should be considered carefully, so the code weaving tool you desire probably isn't necessary. Use Microsoft's libraries as your example, see which methods that implement actually throw <code>ObjectDisposedException</code> when the type implements <code>IDisposable</code>.</p> <p><strong>Note:</strong> <code>GC.SuppressFinalize(this);</code> isn't strictly needed since there is no finalizer, but it's left there since a subclass may implement a finalizer. If the class was marked as <code>sealed</code> then <code>GC.SupressFinalize</code> could be safely removed.</p>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload