Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connect.GetDomains throw System.BadImageFormatException #4

Open
WenceyWang opened this issue Nov 23, 2020 · 9 comments
Open

Connect.GetDomains throw System.BadImageFormatException #4

WenceyWang opened this issue Nov 23, 2020 · 9 comments
Labels
bug Something isn't working

Comments

@WenceyWang
Copy link
Contributor

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
Copy link
Contributor Author

https://github.com/libvirt/libvirt-csharp/blob/c5a8a21027f52450aff24eeba5db0d49316bd133/NativeFunctions.cs#L30 tried to import free from msvcrt.dll or libc.so.6 by dllmap.

@WenceyWang
Copy link
Contributor Author

WenceyWang commented Nov 23, 2020

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 falox added the bug Something isn't working label Nov 24, 2020
@falox
Copy link
Owner

falox commented Nov 24, 2020

@WenceyWang the bug is on Windows only?

@WenceyWang
Copy link
Contributor Author

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

@falox
Copy link
Owner

falox commented Nov 25, 2020

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

@WenceyWang
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

And

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

will cause memory leak.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants