2010-04-02 Miguel de Icaza <miguel@novell.com>

        * SafeHandle.cs: Use refcount == -1 to indicate that the handle
        had been closed.

        The problem here is that we have no records of why the code that
        threw a ObjectDisposedException inside the do { } while block on
        DangerousAddRef was added.   It seems it was added in response to
        Close() methods being called and the need to throw that exception
        on further calls to ObjectDisposedException.

        Sadly, we have no test suites for that.

svn path=/branches/mono-2-6/mcs/; revision=154742

3 files changed, 55 insertions, 5 deletions

diff --git a/mcs/class/corlib/System.Runtime.InteropServices/ChangeLog b/mcs/class/corlib/System.Runtime.InteropServices/ChangeLog
index a67e2c82fa0..de6ee0f34c4 100644
--- a/mcs/class/corlib/System.Runtime.InteropServices/ChangeLog
+++ b/mcs/class/corlib/System.Runtime.InteropServices/ChangeLog
@@ -1,3 +1,16 @@
+2010-04-02  Miguel de Icaza  <miguel@novell.com>
+
+	* SafeHandle.cs: Use refcount == -1 to indicate that the handle
+	had been closed.
+
+	The problem here is that we have no records of why the code that
+	threw a ObjectDisposedException inside the do { } while block on
+	DangerousAddRef was added.   It seems it was added in response to
+	Close() methods being called and the need to throw that exception
+	on further calls to ObjectDisposedException.
+
+	Sadly, we have no test suites for that.
+
 2009-10-15  Sebastien Pouliot  <sebastien@ximian.com>
 
 	* GCHandle.cs: Add missing validations
diff --git a/mcs/class/corlib/System.Runtime.InteropServices/SafeHandle.cs b/mcs/class/corlib/System.Runtime.InteropServices/SafeHandle.cs
index 5a5ef2c5f1e..2873ce4a983 100644
--- a/mcs/class/corlib/System.Runtime.InteropServices/SafeHandle.cs
+++ b/mcs/class/corlib/System.Runtime.InteropServices/SafeHandle.cs
@@ -91,6 +91,7 @@ namespace System.Runtime.InteropServices
 			if (newcount == 0 && owns_handle && !IsInvalid){
 				ReleaseHandle ();
 				handle = invalid_handle_value;
+				refcount = -1;
 			}
 		}
 
@@ -106,7 +107,7 @@ namespace System.Runtime.InteropServices
 		[ReliabilityContract (Consistency.WillNotCorruptState, Cer.MayFail)]
 		public void DangerousAddRef (ref bool success)
 		{
-			if (refcount == 0)
+			if (refcount <= 0)
 				throw new ObjectDisposedException (GetType ().FullName);
 
 			int newcount, current;
@@ -114,7 +115,7 @@ namespace System.Runtime.InteropServices
 				current = refcount;
 				newcount = current + 1;
 				
-				if (handle == invalid_handle_value || current == 0){
+				if (current <= 0){
 					//
 					// In MS, calling sf.Close () followed by a call
 					// to P/Invoke with SafeHandles throws this, but
@@ -130,7 +131,7 @@ namespace System.Runtime.InteropServices
 		[ReliabilityContract (Consistency.WillNotCorruptState, Cer.Success)]
 		public IntPtr DangerousGetHandle ()
 		{
-			if (refcount == 0){
+			if (refcount <= 0){
 				throw new ObjectDisposedException (GetType ().FullName);
 			}
 
@@ -140,7 +141,7 @@ namespace System.Runtime.InteropServices
 		[ReliabilityContract (Consistency.WillNotCorruptState, Cer.Success)]
 		public void DangerousRelease ()
 		{
-			if (refcount == 0)
+			if (refcount <= 0)
 				throw new ObjectDisposedException (GetType ().FullName);
 
 			int newcount, current;
@@ -199,7 +200,7 @@ namespace System.Runtime.InteropServices
 		public bool IsClosed {
 			[ReliabilityContract (Consistency.WillNotCorruptState, Cer.Success)]
 			get {
-				return refcount == 0;
+				return refcount <= 0;
 			}
 		}
 
diff --git a/mcs/class/corlib/Test/System.Runtime.InteropServices/SafeHandleTest.cs b/mcs/class/corlib/Test/System.Runtime.InteropServices/SafeHandleTest.cs
index 9cedc70de8d..cc2488a647c 100644
--- a/mcs/class/corlib/Test/System.Runtime.InteropServices/SafeHandleTest.cs
+++ b/mcs/class/corlib/Test/System.Runtime.InteropServices/SafeHandleTest.cs
@@ -87,6 +87,42 @@ namespace MonoTests.System.Runtime.InteropServices
 			((IDisposable) sf).Dispose ();
 			Assert.AreEqual (sf.released, false, "r3");
 		}
+
+		//
+		// This test does a DangerousAddRef on a new instance
+		// of a custom user Safe Handle, and it just happens
+		// that the default value for the handle is an invalid
+		// handle.
+		//
+		// .NET does not throw an exception in this case, so
+		// we should not either
+		//
+		[Test]
+		public void DangerousAddRefOnNewInstance ()
+		{
+			var h = new IntPtrSafe ();
+			var success = false;
+			h.DangerousAddRef (ref success);
+			Assert.AreEqual (success, true, "daroni");
+		}
+		
+		public class IntPtrSafe : SafeHandle {
+			public IntPtrSafe() : base(IntPtr.Zero, true)
+			{
+			}
+
+			protected override bool ReleaseHandle()
+			{
+				return true;
+			}
+
+			public IntPtr Handle { get; set; }
+
+			public override bool IsInvalid
+			{
+				get { return Handle == IntPtr.Zero; }
+			}
+		}
 	}
 }