Manually generate signatures for some PInvokes to handle SetLastError.

[jlaanstra]

Mimicks the generated code from LibraryImport but with handrolled Get/SetLastError as it's not available everywhere.

[dongle-the-gadget]

CsWinRT/src/cswinrt/strings/WinRT.cs

Lines 32 to 36 in cc36846

	[DllImportAttribute("kernel32.dll", EntryPoint = "SetLastError", ExactSpelling = true)]
	internal static extern void SetLastError(int errorCode);
	[DllImportAttribute("kernel32.dll", EntryPoint = "GetLastError", ExactSpelling = true)]
	internal static extern int GetLastError();

Shouldn't these signatures translate to uint?

[Sergio0694]

As a general rule: if a library is multi-targeting, unless very extreme cases where not possible, do not make compromises on the latest target by using downlevel APIs everywhere. Instead, add necessary polyfills only on the downlevel target, and use the best approach on the latest one.

[Sergio0694]

My two cents: like with previous changes in this file, I'm not really sure this is worth the effort at all right now, as this is not code we should keep. We need to bump the TFM for CsWinRT to .NET 8 (I guess once it goes GA), and then only handle NS2.0 and .NET 8, where on the .NET 8 we'll be able to just use [LibraryImport], which will do the right thing. For downlevel, we can then either not worry about eg. the last system error, or just add polyfills then. But making so many changes now I think is not ideal, given we'll end up rewriting most of this file soon anyway once we finally can bump that TFM 🤔

[DaZombieKiller]

.NET 6 introduced the following APIs in order to aid scenarios involving LastError values (proposal here):

namespace System.Runtime.InteropServices;

public static partial class Marshal
{
    public static int GetLastSystemError();
    public static int GetLastPInvokeError();
    public static void SetLastSystemError(int error);
    public static void SetLastPInvokeError(int error);
}

The point of these APIs was not simply convenience either. Prior to .NET 6, it was not possible to directly call GetLastError or SetLastError safely. This is because the runtime offered no guarantee that it would not call SetLastError on your behalf (directly or otherwise) between your P/Invoke and your own GetLastError call (this is why SetLastError exists on DllImport).

To solve this, when your P/Invoke is annotated with SetLastError = true, the runtime would cache the result of GetLastError as part of the marshalling stub after the native call, and provided access to it through Marshal.GetLastWin32Error (which is now an alias for GetLastPInvokeError).

In short, you should never manually P/Invoke GetLastError or SetLastError directly in .NET code, if you can avoid it.

The changes proposed in this PR do not account for this behavior, and are therefore unsafe on targets older than .NET 6. The correct solution is to conditionally use LibraryImport or DllImport with SetLastError = true:

#if NET7_0_OR_GREATER
[LibraryImport("kernel32.dll", SetLastError = true)]
internal static unsafe partial IntPtr LoadLibraryExW(ushort* fileName, IntPtr fileHandle, uint flags);
#else
[DllImport("kernel32.dll", SetLastError = true)]
internal static unsafe extern IntPtr LoadLibraryExW(ushort* fileName, IntPtr fileHandle, uint flags);
#endif

Though as you can see, this only takes effect on .NET 7 and higher (because LibraryImport was only introduced in .NET 7). For .NET 6, you would need an additional block mimicking the LibraryImport code, but using the proper LastError APIs. At that point, I have to wonder if it's a change worth pursuing when you consider the risks.

[dongle-the-gadget]

We need to bump the TFM for CsWinRT to .NET 8 (I guess once it goes GA)

@manodasanW plans on moving away from the .NET 6 TFM?

[jlaanstra]

I'll update the PR with multitargeting given some of the concerns raised about doing this correct downlevel.

I don't see .NET 6 getting dropped soon.

[jlaanstra]

As a general rule: if a library is multi-targeting, unless very extreme cases where not possible, do not make compromises on the latest target by using downlevel APIs everywhere. Instead, add necessary polyfills only on the downlevel target, and use the best approach on the latest one.

Changed to only use new signatures on .NET 6 and newer.

[manodasanW]

We will need to continue supporting .NET 6 as it is an LTS. But I am exploring what it means to have both .NET 6 and .NET 8 versions of the Windows SDK projection. With that said, given we are a targeting pack, it is looking like it is going to require .NET SDK changes first before that can be done so I won't take a dependency on that for now.