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=/trunk/mcs/; revision=154741

diff --git a/mcs/class/corlib/System.Runtime.InteropServices/ChangeLog b/mcs/class/corlib/System.Runtime.InteropServices/ChangeLog
index 05c70839ba2196169a857f4f6eabc12589ebcc6e..defb099c802f0837b177fffac4b8be3e25e645d4 100644 (file)
--- 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.
+
 2010-03-16  Jb Evain  <jbevain@novell.com>
 
        * MarshalAsAttribute.cs, Marshal.cs: use MOONLIGHT symbol to

diff --git a/mcs/class/corlib/System.Runtime.InteropServices/SafeHandle.cs b/mcs/class/corlib/System.Runtime.InteropServices/SafeHandle.cs
index 028e8f0b426e4f84cc9b6f18ba1558ad79323467..e1b0a70ffb24ddc408c4b20af7f1c0b81ae2814f 100644 (file)
--- a/mcs/class/corlib/System.Runtime.InteropServices/SafeHandle.cs
+++ b/mcs/class/corlib/System.Runtime.InteropServices/SafeHandle.cs
@@ -90,6 +90,7 @@ namespace System.Runtime.InteropServices
                        if (newcount == 0 && owns_handle && !IsInvalid){
                                ReleaseHandle ();
                                handle = invalid_handle_value;
+                               refcount = -1;
                        }
                }
 
@@ -105,7 +106,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;
@@ -113,7 +114,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
@@ -129,7 +130,7 @@ namespace System.Runtime.InteropServices
                [ReliabilityContract (Consistency.WillNotCorruptState, Cer.Success)]
                public IntPtr DangerousGetHandle ()
                {
-                       if (refcount == 0){
+                       if (refcount <= 0){
                                throw new ObjectDisposedException (GetType ().FullName);
                        }
 
@@ -139,7 +140,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;
@@ -198,7 +199,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 9cedc70de8d81bbec892b1cc013752953cd07bbe..cc2488a647cd9a3f127a5a9491549966298c46ba 100644 (file)
--- 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; }
+                       }
+               }
        }
 }