Add Kernel32 and User32 methods, and tests #329

[AArnott]

I added some methods (including those required to test #329) and added a test to reproduce #329. But the test passed so I don't think we actually have an issue, unless @AkariAkaori can help point out what is wrong in my repro.

Closes #329

[vbfox]

I suspect that the exact error @AkariAkaori has to be platform related (Some OS version might setlasterror on success, other not. So we should fix our GetWindowText implementation to call SetLastError (And cross fingers that the framework doesn't interrupt our tread to run some GC code that change last error, no idea if such code exists, we could block that by being in a CER)

CoreClr did the wise thing here and guarantee that SetLastError is always called on PInvoke having SetLastError=true https://github.com/dotnet/coreclr/pull/614

[AArnott]

has to be platform related (Some OS version might setlasterror on success, other not.

Yes, that occurred to me as well. It could be .NET Framework that fixed this too (an equivalent fix that CoreCLR made). If @AkariAkaori can specify what version of .NET Framework is installed and what build of Windows, I'd be willing to investigate more. But I don't want to fix a problem that I can't repro since my fix may or may not be effective.

@vbfox I've never written a CER in C# (unless you count locks). What construct would protect against interruptions? BTW, as the last error is thread-local, I imagine as long as I don't allocate memory, it's impossible (I think?) for the CLR to actually do work on my thread so I think(?) my LastError is safe if I were to set it to zero.

[vbfox]

I've only used CER in a few specific cases and i'm far from knowledgeable in the domain but yes they protect against all interruptions by the runtime as long as you respect a big amount of constraints

RuntimeHelpers.PrepareConstrainedRegions();
try { }
finally
{
    // Code Here won't be interruptible
}

I agree with you that it should be an impossible situation in the current CRL implementations. Especially as I don't think that the GC really uses code that set the win32 last error. AFAIK nothing that currently exists on the CLR run code on random threads except for the GC and none of the current algorithm run code on a thread that is not allocating.

I was pointing at the possibility because while I think that if we can find a case were it's provably broken we should fix it by adding a SetLastError the correct fix is to do it in the runtime like coreclr did as it's the only place that can be sure that the call is really the last thing done before calling the PInvoked method.

[AArnott]

And cross fingers that the framework doesn't interrupt our tread to run some GC code that change last error, no idea if such code exists, we could block that by being in a CER

Thinking about this more, I see no reason to worry about using a CER. That's why the Framework gives us Marshal.GetLastWin32Error() after all (it ensures that even if the CLR interrupts and changes GetLastError's result, you still get the last one that your code would care about on that thread. And that's the method we're using.

So IMO it's still a debate whether we need to call SetLastError(ERROR_SUCCESS) before calling GetWindowTextLength since if a version of .NET or Windows didn't call that for us when returning 0 length I agree that would be an issue. But I don't like fixing bugs that don't repro. So unless @AkariAkaori can suggest an OS or .NET version to test on to repro, I'll close that bug as not repro.

[vbfox]

Thinking about this more, I see no reason to worry about using a CER. That's why the Framework gives us Marshal.GetLastWin32Error() after all (it ensures that even if the CLR interrupts and changes GetLastError's result, you still get the last one that your code would care about on that thread. And that's the method we're using.

My point was more about the time between our hypothetical call to SetLastError and the next pinvoke. But your second point stand fixing bugs that we can't repro is problematic :)

[gabbyzhat]

Managed to reproduce it here: https://streamable.com/gdkio The magic number is a one of the windows with blank text Only crashes if the call happens after Console.WriteLine

Tested latest PInvoke 0.5.86 and 0.5.64, and frameworks 4.5.2, 4.6.2 and 4.7