falox/libvirt-csharp

Connect.GetDomains throw System.BadImageFormatException

WenceyWang opened this issue · 9 comments

WenceyWang commented 
public List<Domain> GetDomains(virConnectListAllDomainsFlags flags = default)
{
    int result = Libvirt.virConnectListAllDomains(_conn, out IntPtr ptrDomains, flags);

    ThrowExceptionOnError(result);

    List<Domain> domains = new List<Domain>();

    for (int i = 0; i < result; i++)
    {
        IntPtr ptrDomain = Marshal.ReadIntPtr(ptrDomains, i * IntPtr.Size);
        domains.Add(new Domain(_conn, ptrDomain));
    }

    Marshal.FreeHGlobal(ptrDomains);

    return domains;
}

Marshal.FreeHGlobal(ptrDomains); calls Kernel32.LocalFree instead of free in vcrt on Windows and cause System.BadImageFormatException

WenceyWang commented

image
I have found a VirFree in prebuilt dll exports, It's

/**
 * VIR_FREE:
 * @ptr: pointer holding address to be freed
 *
 * Free the memory stored in 'ptr' and update to point
 * to NULL.
 *
 * This macro is safe to use on arguments with side effects.
 */
#define VIR_FREE(ptr) g_clear_pointer(&(ptr), g_free)

and in driver implemention:

if (domains) {
        doms = g_new0(virDomainPtr, nvms + 1);

        for (i = 0; i < nvms; i++) {
            virDomainObjPtr vm = vms[i];

            virObjectLock(vm);
            doms[i] = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
            virObjectUnlock(vm);

            if (!doms[i])
                goto cleanup;
        }

        *domains = doms;
        doms = NULL;
    }

which memory is allocated by https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-new0

falox commented

@WenceyWang the bug is on Windows only?

WenceyWang commented

seems yes, Marshal.FreeHGlobal will call free in libc.

falox commented

So, if you comment Marshal.FreeHGlobal(ptrDomains); and run the tests on Windows with dotnet test, is everything ok?

WenceyWang commented

For GetDomains, Yes. I have P/Invoked virFree to free this memory and things go fine.

But for other tests. It's not so well. there are a lot of problems, Domain.Xml have some issue and much more.

I'll PR after I fixed things up.

libvirt.LibvirtException
invalid domain pointer in virDomainGetXMLDesc
   at libvirt.LibvirtHelper.ThrowExceptionOn(Func`1 predicate) in C:\Repos\libvirt-csharp-core\src\libvirt\LibvirtHelper.cs:line 22
   at libvirt.LibvirtHelper.ThrowExceptionOnError(String result) in C:\Repos\libvirt-csharp-core\src\libvirt\LibvirtHelper.cs:line 12
   at libvirt.LibvirtObject.ThrowExceptionOnError(String result) in C:\Repos\libvirt-csharp-core\src\libvirt\LibvirtObject.cs:line 58
   at libvirt.LibvirtObject.GetString(Func`1 func) in C:\Repos\libvirt-csharp-core\src\libvirt\LibvirtObject.cs:line 25
   at libvirt.Domain.get_Xml() in C:\Repos\libvirt-csharp-core\src\libvirt\Domain.cs:line 50
WenceyWang commented

I found that the problem is that Marshal to string:

[DllImport(Libvirt.Name, CallingConvention = CallingConvention.Cdecl, EntryPoint = "virDomainGetXMLDesc")]
[return: MarshalAs(UnmanagedType.LPStr)]
public static extern string virDomainGetXMLDesc(IntPtr domain, int flags = 0);

https://stackoverflow.com/questions/370079/pinvoke-for-c-function-that-returns-char/370519

WenceyWang commented

I think we have to read strings using Marshal.PtrToStringAuto and then free memory by P/Invoke free in msvcrt/libc.

And virFree does not exist on dll I complied. We have to DllImport free from msvcrt/libc.

WenceyWang commented

And

public IntPtr MarshalManagedToNative(object ManagedObj) =>  Marshal.StringToHGlobalAnsi((string) ManagedObj);

will cause memory leak.