[libvirt] [PATCH C#] Fix two memory leaks in the ConnectCredential.Result setter
StringToHGlobalAnsi returns a pointer to unmanaged memory that must be freed using FreeHGlobal. When the setter is called twice the strdup'ed unmanaged string from the first call leaks. Free it before assigning it again. --- NativeFunctions.cs | 6 +++++- Types.cs | 13 +++++++++++-- projects/MonoDevelop/LibvirtBindings.dll.config | 2 ++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/NativeFunctions.cs b/NativeFunctions.cs index 25d7532..71136b8 100644 --- a/NativeFunctions.cs +++ b/NativeFunctions.cs @@ -16,8 +16,12 @@ namespace Libvirt { public class NativeFunctions { - // TODO : this is atemporary workaround for virConnectOpenAuth callback, this should be removed + // TODO : this is a temporary workaround for virConnectOpenAuth callback, this should be removed [DllImport("msvcrt.dll", EntryPoint = "_strdup", CallingConvention = CallingConvention.Cdecl)] public static extern IntPtr StrDup(IntPtr strSource); + + // TODO : this is a temporary workaround for virConnectOpenAuth callback, this should be removed + [DllImport("msvcrt.dll", EntryPoint = "free", CallingConvention = CallingConvention.Cdecl)] + public static extern void Free(IntPtr ptr); } } diff --git a/Types.cs b/Types.cs index 0d471fb..37a1f47 100644 --- a/Types.cs +++ b/Types.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (C) * Arnaud Champion <arnaud.champion@devatom.fr> * Jaromír Červenka <cervajz@cervajz.com> @@ -406,6 +406,10 @@ namespace Libvirt [StructLayout(LayoutKind.Sequential)] public class ConnectCredential { + public ConnectCredential () + { + result = IntPtr.Zero; + } ///<summary> /// One of virConnectCredentialType constants ///</summary> @@ -473,8 +477,13 @@ namespace Libvirt } set { - result = NativeFunctions.StrDup(Marshal.StringToHGlobalAnsi(value)); + IntPtr tmp = Marshal.StringToHGlobalAnsi(value); + + NativeFunctions.Free(result); + result = NativeFunctions.StrDup(tmp); resultlen = (uint)value.Length; + + Marshal.FreeHGlobal(tmp); } } } diff --git a/projects/MonoDevelop/LibvirtBindings.dll.config b/projects/MonoDevelop/LibvirtBindings.dll.config index d0ded4c..2dfa2e4 100644 --- a/projects/MonoDevelop/LibvirtBindings.dll.config +++ b/projects/MonoDevelop/LibvirtBindings.dll.config @@ -6,6 +6,8 @@ </dllmap> <dllmap dll="msvcrt.dll"> <dllentry os="windows" dll="msvcrt.dll" name="_strdup" target="_strdup" /> + <dllentry os="windows" dll="msvcrt.dll" name="free" target="free" /> <dllentry os="linux" dll="libc.so.6" name="_strdup" target="strdup" /> + <dllentry os="linux" dll="libc.so.6" name="free" target="free" /> </dllmap> </configuration> -- 1.7.0.4
On Thu, Oct 28, 2010 at 12:38:17PM +0200, Matthias Bolte wrote:
StringToHGlobalAnsi returns a pointer to unmanaged memory that must be freed using FreeHGlobal.
When the setter is called twice the strdup'ed unmanaged string from the first call leaks. Free it before assigning it again. [...] + + // TODO : this is a temporary workaround for virConnectOpenAuth callback, this should be removed + [DllImport("msvcrt.dll", EntryPoint = "free", CallingConvention = CallingConvention.Cdecl)] + public static extern void Free(IntPtr ptr); } [...] + IntPtr tmp = Marshal.StringToHGlobalAnsi(value); + + NativeFunctions.Free(result); + result = NativeFunctions.StrDup(tmp); resultlen = (uint)value.Length; + + Marshal.FreeHGlobal(tmp);
This raises 2 questions, how temporary is 'temporary' ;-) ? And I assume the client code don't need to do similar things, that's just the bindings, right ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
?Hi, this code is only for the bindings, not the client code. I try to find a solution since times now, but I haven't find an acceptable one for now. I keep searching anyway. Arnaud -------------------------------------------------- From: "Daniel Veillard" <veillard@redhat.com> Sent: Thursday, October 28, 2010 9:32 PM To: "Matthias Bolte" <matthias.bolte@googlemail.com> Cc: <libvir-list@redhat.com> Subject: Re: [libvirt] [PATCH C#] Fix two memory leaks in the ConnectCredential.Result setter
On Thu, Oct 28, 2010 at 12:38:17PM +0200, Matthias Bolte wrote:
StringToHGlobalAnsi returns a pointer to unmanaged memory that must be freed using FreeHGlobal.
When the setter is called twice the strdup'ed unmanaged string from the first call leaks. Free it before assigning it again. [...] + + // TODO : this is a temporary workaround for virConnectOpenAuth callback, this should be removed + [DllImport("msvcrt.dll", EntryPoint = "free", CallingConvention = CallingConvention.Cdecl)] + public static extern void Free(IntPtr ptr); } [...] + IntPtr tmp = Marshal.StringToHGlobalAnsi(value); + + NativeFunctions.Free(result); + result = NativeFunctions.StrDup(tmp); resultlen = (uint)value.Length; + + Marshal.FreeHGlobal(tmp);
This raises 2 questions, how temporary is 'temporary' ;-) ? And I assume the client code don't need to do similar things, that's just the bindings, right ?
Daniel
-- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Oct 29, 2010 at 09:27:56AM +0200, arnaud.champion@devatom.fr wrote:
?Hi,
this code is only for the bindings, not the client code. I try to find a solution since times now, but I haven't find an acceptable one for now. I keep searching anyway.
Okay I was just wondering :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
arnaud.champion＠devatom.fr -
Daniel Veillard -
Matthias Bolte