Re: [Mono-dev] Finalizers in CriticalHandle
On Sat, 2011-01-22 at 00:09 -0500, Gonzalo Paniagua Javier wrote: I think a more correct patch would be the one attached. Avoids having 'if (Invalid) return;' followed by 'if (!Invalid)...'. Sure. I just wanted to highlight the obviousness of the change. I'll commit your version. Miguel has asked me to commit it to 2-10 and master. Should I add it to 2-8 as well? - Dick ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Finalizers in CriticalHandle
On 24.01.2011 11:11, Dick Porter wrote: On Sat, 2011-01-22 at 00:09 -0500, Gonzalo Paniagua Javier wrote: I think a more correct patch would be the one attached. Avoids having 'if (Invalid) return;' followed by 'if (!Invalid)...'. Sure. I just wanted to highlight the obviousness of the change. I'll commit your version. Gonzalo has already committed this to master (a9d2a487) 2-10 (234a2d72). Robert ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Finalizers in CriticalHandle
On Mon, 2011-01-24 at 11:22 +0100, Robert Jordan wrote: On 24.01.2011 11:11, Dick Porter wrote: Sure. I just wanted to highlight the obviousness of the change. I'll commit your version. Gonzalo has already committed this to master (a9d2a487) 2-10 (234a2d72). Oops, missed that. Saves me a task then :) - Dick ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Finalizers in CriticalHandle
Patch looks good to me. On Sat, Jan 22, 2011 at 6:09 AM, Gonzalo Paniagua Javier gonzalo.m...@gmail.com wrote: On Thu, 2011-01-20 at 16:15 +, Dick Porter wrote: On 17 Jan 2011, at 6:00PM, Rodrigo Kumpera wrote: I'm not sure what the defined behavior is on this case, MSDN is not always accurate and we need to be as compatible with MS as possible. Could you please write a test case, and if the behavior you suggest is correct, send a patch which includes both the fix and the test case so we can move forward? Please see attached test case and diff. I think a more correct patch would be the one attached. Avoids having 'if (Invalid) return;' followed by 'if (!Invalid)...'. -Gonzalo ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Finalizers in CriticalHandle
On Thu, 2011-01-20 at 16:15 +, Dick Porter wrote:
On 17 Jan 2011, at 6:00PM, Rodrigo Kumpera wrote:
I'm not sure what the defined behavior is on this case, MSDN is not
always accurate and we
need to be as compatible with MS as possible.
Could you please write a test case, and if the behavior you suggest
is correct, send a patch
which includes both the fix and the test case so we can move forward?
Please see attached test case and diff.
I think a more correct patch would be the one attached. Avoids having
'if (Invalid) return;' followed by 'if (!Invalid)...'.
-Gonzalo
diff --git a/mcs/class/corlib/System.Runtime.InteropServices/CriticalHandle.cs b/mcs/class/corlib/System.Runtime.InteropServices/CriticalHandle.cs
index 479dc44..ce50aff 100644
--- a/mcs/class/corlib/System.Runtime.InteropServices/CriticalHandle.cs
+++ b/mcs/class/corlib/System.Runtime.InteropServices/CriticalHandle.cs
@@ -47,17 +47,14 @@ namespace System.Runtime.InteropServices
if (_disposed)
return;
- _disposed = true;
- if (IsInvalid)
-return;
-
- if (disposing == true !IsInvalid){
-if (!ReleaseHandle ()) {
+ if (!IsInvalid){
+if (!_disposed !ReleaseHandle ()) {
GC.SuppressFinalize (this);
} else {
// Failed in release...
}
}
+ _disposed = true;
}
[ReliabilityContract (Consistency.WillNotCorruptState, Cer.Success)]
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Finalizers in CriticalHandle
On 17 Jan 2011, at 6:00PM, Rodrigo Kumpera wrote: I'm not sure what the defined behavior is on this case, MSDN is not always accurate and we need to be as compatible with MS as possible. Could you please write a test case, and if the behavior you suggest is correct, send a patch which includes both the fix and the test case so we can move forward? Please see attached test case and diff. Test case on windows: C:\Documents and Settings\Administrator\Desktop\CriticalHandle \CriticalHandle\bin\DebugCriticalHandle.exe Handle 8 was released! Handle 3 was released! Handle 2 was released! Handle 1 was released! Handle 0 was released! Handle 7 was released! Handle 6 was released! Handle 5 was released! Handle 4 was released! Handle 9 was released! C:\Documents and Settings\Administrator\Desktop\CriticalHandle \CriticalHandle\bin\Debug Test case on unpatched mono, git master as of this afternoon: [dick@hagbard /tmp] [ 3:53PM] Mono HEAD :; mono CriticalHandleTest.exe [dick@hagbard /tmp] [ 3:53PM] Mono HEAD :; mono-sgen CriticalHandleTest.exe [dick@hagbard /tmp] [ 3:54PM] Mono HEAD :; Test case on patched mono: [dick@hagbard /tmp] [ 4:00PM] Mono HEAD :; mono CriticalHandleTest.exe Handle 7 was released! Handle 1 was released! Handle 6 was released! Handle 0 was released! Handle 5 was released! Handle 4 was released! Handle 3 was released! Handle 2 was released! Handle 8 was released! Handle 9 was released! [dick@hagbard /tmp] [ 4:00PM] Mono HEAD :; mono-sgen CriticalHandleTest.exe Handle 8 was released! Handle 7 was released! Handle 5 was released! Handle 6 was released! Handle 3 was released! Handle 4 was released! Handle 2 was released! Handle 0 was released! Handle 1 was released! Handle 9 was released! [dick@hagbard /tmp] [ 4:00PM] Mono HEAD :; - Dick CriticalHandleTest.cs Description: Binary data criticalhandle.diff Description: Binary data ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Finalizers in CriticalHandle
Hello Dick, Please apply to both 2-10 and master branches. Thanks for tracking this down. Miguel On Thu, Jan 20, 2011 at 11:15 AM, Dick Porter dpor...@codicesoftware.com wrote: On 17 Jan 2011, at 6:00PM, Rodrigo Kumpera wrote: I'm not sure what the defined behavior is on this case, MSDN is not always accurate and we need to be as compatible with MS as possible. Could you please write a test case, and if the behavior you suggest is correct, send a patch which includes both the fix and the test case so we can move forward? Please see attached test case and diff. Test case on windows: C:\Documents and Settings\Administrator\Desktop\CriticalHandle\CriticalHandle\bin\DebugCriticalHandle.exe Handle 8 was released! Handle 3 was released! Handle 2 was released! Handle 1 was released! Handle 0 was released! Handle 7 was released! Handle 6 was released! Handle 5 was released! Handle 4 was released! Handle 9 was released! C:\Documents and Settings\Administrator\Desktop\CriticalHandle\CriticalHandle\bin\Debug Test case on unpatched mono, git master as of this afternoon: [dick@hagbard /tmp] [ 3:53PM] Mono HEAD :; mono CriticalHandleTest.exe [dick@hagbard /tmp] [ 3:53PM] Mono HEAD :; mono-sgen CriticalHandleTest.exe [dick@hagbard /tmp] [ 3:54PM] Mono HEAD :; Test case on patched mono: [dick@hagbard /tmp] [ 4:00PM] Mono HEAD :; mono CriticalHandleTest.exe Handle 7 was released! Handle 1 was released! Handle 6 was released! Handle 0 was released! Handle 5 was released! Handle 4 was released! Handle 3 was released! Handle 2 was released! Handle 8 was released! Handle 9 was released! [dick@hagbard /tmp] [ 4:00PM] Mono HEAD :; mono-sgen CriticalHandleTest.exe Handle 8 was released! Handle 7 was released! Handle 5 was released! Handle 6 was released! Handle 3 was released! Handle 4 was released! Handle 2 was released! Handle 0 was released! Handle 1 was released! Handle 9 was released! [dick@hagbard /tmp] [ 4:00PM] Mono HEAD :; - Dick ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Finalizers in CriticalHandle
On Fri, 2011-01-14 at 17:26 +0100, Rodrigo Kumpera wrote: I suppose it is. Does .NET call Release on finalizer? I've no idea, but is there any reason why it shouldn't? It is after all supposed to be there for cleaning up unmanaged resources. - Dick ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Finalizers in CriticalHandle
On Mon, Jan 17, 2011 at 5:19 PM, Dick Porter dpor...@codicesoftware.comwrote: On Fri, 2011-01-14 at 17:26 +0100, Rodrigo Kumpera wrote: I suppose it is. Does .NET call Release on finalizer? I've no idea, but is there any reason why it shouldn't? It is after all supposed to be there for cleaning up unmanaged resources. I'm not sure what the defined behavior is on this case, MSDN is not always accurate and we need to be as compatible with MS as possible. Could you please write a test case, and if the behavior you suggest is correct, send a patch which includes both the fix and the test case so we can move forward? Cheers, Rodrigo ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
[Mono-dev] Finalizers in CriticalHandle
Hi all
I'm currently debugging an issue that appears to be caused by the
non-release of unmanaged resources in CriticalHandle. I'm using the git
master branch.
In CriticalHandle.cs:
[ReliabilityContract (Consistency.WillNotCorruptState,
Cer.Success)]
~CriticalHandle ()
{
Dispose (false);
}
[ReliabilityContract (Consistency.WillNotCorruptState,
Cer.Success)]
protected virtual void Dispose (bool disposing)
{
if (_disposed)
return;
_disposed = true;
if (IsInvalid)
return;
if (disposing == true !IsInvalid){
if (!ReleaseHandle ()) {
GC.SuppressFinalize (this);
} else {
// Failed in release...
}
}
}
Unless I'm missing something, this looks to me that when the
CriticalHandle object is finalized, the unmanaged resources won't be
released: ReleaseHandle() isn't called.
Is this a bug?
- Dick
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Finalizers in CriticalHandle
I suppose it is. Does .NET call Release on finalizer?
On Fri, Jan 14, 2011 at 4:30 PM, Dick Porter dpor...@codicesoftware.comwrote:
Hi all
I'm currently debugging an issue that appears to be caused by the
non-release of unmanaged resources in CriticalHandle. I'm using the git
master branch.
In CriticalHandle.cs:
[ReliabilityContract (Consistency.WillNotCorruptState,
Cer.Success)]
~CriticalHandle ()
{
Dispose (false);
}
[ReliabilityContract (Consistency.WillNotCorruptState,
Cer.Success)]
protected virtual void Dispose (bool disposing)
{
if (_disposed)
return;
_disposed = true;
if (IsInvalid)
return;
if (disposing == true !IsInvalid){
if (!ReleaseHandle ()) {
GC.SuppressFinalize (this);
} else {
// Failed in release...
}
}
}
Unless I'm missing something, this looks to me that when the
CriticalHandle object is finalized, the unmanaged resources won't be
released: ReleaseHandle() isn't called.
Is this a bug?
- Dick
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
