Bug? PEBCAK? Unable to initialize __nuint_1 to user-defined length

skst commented 1 year ago

Actual behavior

Full disclosure: I am probably missing something or do not know how to do something (although, I have spent the day reading and researching and testing).

When calling an API and passing a structure which contains a variable array, CsWin32 generates a struct that has a member of type __nuint_1. However, I see no way to set the __nuint_1's SpanLength so that it allocates a specific number of elements.

Expected behavior

It seems that __nuint_1 should have a way to initialize it to have a user-specified number of elements, since we have to pass the size of the memory block to the function.

Repro steps

NativeMethods.txt content:

QueryInformationJobObject
JOBOBJECT_BASIC_PROCESS_ID_LIST

Generated:

internal partial struct JOBOBJECT_BASIC_PROCESS_ID_LIST
{
    internal uint NumberOfAssignedProcesses;
    internal uint NumberOfProcessIdsInList;
    internal winmdroot.__nuint_1 ProcessIdList;
}

internal partial struct __nuint_1
{
    private const int SpanLength = 1;

    /// <summary>The length of the inline array.</summary>
    internal readonly int Length => SpanLength;
    internal nuint _0;

    /// <summary>
    /// Gets this inline array as a span.
    /// </summary>
    /// <remarks>
    /// ⚠ Important ⚠: When this struct is on the stack, do not let the returned span outlive the stack frame that defines it.
    /// </remarks>
    [UnscopedRef]
    internal unsafe Span<nuint> AsSpan() => MemoryMarshal.CreateSpan(ref _0, SpanLength);

    /// <summary>
    /// Gets this inline array as a span.
    /// </summary>
    /// <remarks>
    /// ⚠ Important ⚠: When this struct is on the stack, do not let the returned span outlive the stack frame that defines it.
    /// </remarks>
    [UnscopedRef]
    internal unsafe readonly ReadOnlySpan<nuint> AsReadOnlySpan() => MemoryMarshal.CreateReadOnlySpan(ref Unsafe.AsRef(_0), SpanLength);
    public static implicit operator __nuint_1(ReadOnlySpan<nuint> value)
    {
        Unsafe.SkipInit(out __nuint_1 result);
        value.CopyTo(result.AsSpan());
        int initLength = value.Length;
        result.AsSpan().Slice(initLength, SpanLength - initLength).Clear();
        return result;
    }
}

Any of your own code that should be shared?

JOBOBJECT_BASIC_PROCESS_ID_LIST idList = new();
uint nData = (uint)(Marshal.SizeOf(idList) + Marshal.SizeOf(idList.ProcessIdList) * 100);   // We want to allocate 100 elements
PInvoke.QueryInformationJobObject(hJob, JOBOBJECTINFOCLASS.JobObjectBasicProcessIdList, &idList, nData, &returnLength);

Context

CsWin32 version: 0.2.188-beta
Win32Metadata version (if explicitly set by project):
Target Framework: .NET 7
LangVersion (if explicitly set by project): latest (C# 11)

AArnott commented 1 year ago

The problem is CsWin32 doesn't recognize this as a variable length array, so it just uses a fixed 1-length array.

See #387, which tracks improving the usability of this case.

As a workaround, assuming the struct is initialized elsewhere, you can access the full list like this:

JOBOBJECT_BASIC_PROCESS_ID_LIST list;
unsafe
{
    Span<nuint> pids = new(&list.ProcessIdList._0, (int)list.NumberOfProcessIdsInList);
    for (int i = 0; i < pids.Length; i++)
    {
        Console.WriteLine(pids[i]);
    }
}

But if you need to initialize the list, you'll have to allocate enough memory for the struct, and then pin the struct at that location yourself. I can provide sample code for that if you need.

skst commented 1 year ago

Yes, QueryInformationJobObject requires the caller to allocate the memory, and I have been unsuccessful at coercing a block of memory to be a JOBOBJECT_BASIC_PROCESS_ID_LIST. If you could provide that sample code, I would appreciate it.

I got this far, which seems to work, but might not be the most efficient.

    nint nativeMemory = Marshal.AllocHGlobal(nData);
    JOBOBJECT_BASIC_PROCESS_ID_LIST* pidList = (JOBOBJECT_BASIC_PROCESS_ID_LIST*)nativeMemory.ToPointer();

    PInvoke.QueryInformationJobObject(hJob, JOBOBJECTINFOCLASS.JobObjectBasicProcessIdList, pidList, (uint)nData, &returnLength);

    JOBOBJECT_BASIC_PROCESS_ID_LIST idList = *pidList;
    var ids = idList.ProcessIdList.AsReadOnlySpan();
    foreach (var item in ids)
    {
    }

AArnott commented 1 year ago

JOBOBJECT_BASIC_PROCESS_ID_LIST idList = *pidList;
var ids = idList.ProcessIdList.AsReadOnlySpan();

This is broken for two reasons: AsReadOnlySpan() will only give you the first item. And secondly, because your *pidList expression will copy the struct from native memory onto your stack, without all the elements after the first one.

I'm preparing a sample of what I believe should work.

AArnott commented 1 year ago

using Windows.Win32;
using Windows.Win32.Foundation;
using Windows.Win32.System.JobObjects;

unsafe
{
    HANDLE hJob = default;
    const int MaxProcessCount = 4096; // TODO: pick a good number here
    int MemorySize = sizeof(JOBOBJECT_BASIC_PROCESS_ID_LIST) + MaxProcessCount * sizeof(nuint);
    fixed (byte* pBytes = new byte[MemorySize])
    {
        JOBOBJECT_BASIC_PROCESS_ID_LIST* pList = (JOBOBJECT_BASIC_PROCESS_ID_LIST*)pBytes;

        uint returnLength = 0;
        PInvoke.QueryInformationJobObject(hJob, JOBOBJECTINFOCLASS.JobObjectBasicProcessIdList, pList, (uint)MemorySize, &returnLength);

        Span<nuint> pids = new(&pList->ProcessIdList, (int)pList->NumberOfProcessIdsInList);
        for (int i = 0; i < pids.Length; i++)
        {
            Console.WriteLine(pids[i]);
        }
    }
}

I'm using memory allocated from the GC heap so that you don't have to worry about a try/finally block to ensure you don't leak unmanaged memory.

skst commented 1 year ago

Thank you very much, Andrew. I'll give that a go.

Update: Yep, works great. Thanks again!

microsoft / CsWin32