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

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

Closed
skst opened this issue Mar 4, 2023 · 5 comments
Closed

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

skst opened this issue Mar 4, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@skst
Copy link

skst commented Mar 4, 2023

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

  1. 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;
		}
	}
  1. 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)
@skst skst added the bug Something isn't working label Mar 4, 2023
@AArnott
Copy link
Member

AArnott commented Mar 6, 2023

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.

@AArnott AArnott closed this as not planned Won't fix, can't repro, duplicate, stale Mar 6, 2023
@skst
Copy link
Author

skst commented Mar 14, 2023

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
Copy link
Member

AArnott commented Mar 14, 2023

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
Copy link
Member

AArnott commented Mar 14, 2023

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

skst commented Mar 14, 2023

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

Update: Yep, works great. Thanks again!

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