61

I tested a lot. But I found no disadvantages of those 2!
But see the accepted answer.


I read here that calling GetLastError in managed code is unsafe because the Framework might internally "overwrite" the last error. I have never had any noticeable problems with GetLastError and it seems for me that the .NET Framework is smart enough not to overwrite it. Therefore I have a few questions on that topic:
  • in [DllImport("kernel32.dll", SetLastError = true)] does the SetLastError attribute make the Framework store the error code for the use of Marshal.GetLastWin32Error() ?
  • is there an example where plain GetLastError fails to give the correct result ?
  • do I really HAVE to use Marshal.GetLastWin32Error() ?
  • is this "problem" Framework version related ?

public class ForceFailure
{
    [DllImport("kernel32.dll")]
    static extern uint GetLastError();
    [DllImport("kernel32.dll", SetLastError = true)]
    static extern bool SetVolumeLabel(string lpRootPathName, string lpVolumeName);

    public static void Main()
    {
        if (SetVolumeLabel("XYZ:\\", "My Imaginary Drive "))
            System.Console.WriteLine("It worked???");
        else
        {
            // the first last error check is fine here:
            System.Console.WriteLine(GetLastError());
            System.Console.WriteLine(Marshal.GetLastWin32Error());
        }
    }
}


Producing errors:
if (SetVolumeLabel("XYZ:\\", "My Imaginary Drive "))
    Console.WriteLine("It worked???");
else
{
    // bad programming but ok GetlLastError is overwritten:
    Console.WriteLine(Marshal.GetLastWin32Error());
    try
    {
        using (new FileStream("sdsdafsdfsdfs sdsd ", FileMode.Open)) { }
    }
    catch { }
    Console.WriteLine(GetLastError());
}

if (SetVolumeLabel("XYZ:\\", "My Imaginary Drive "))
    Console.WriteLine("It worked???");
else
{
    // bad programming and Marshal.GetLastWin32Error() is overwritten as well:
    Console.WriteLine(GetLastError());
    try
    {
        using (new FileStream("sdsdafsdfsdfs sdsd ", FileMode.Open)) { }
    }
    catch { }
    Console.WriteLine(Marshal.GetLastWin32Error());
}

// turn off concurrent GC
GC.Collect(); // doesn't effect any of the candidates

Console.WriteLine(" -> " + GetLastError());
Console.WriteLine(" -> " + GetLastError());
Console.WriteLine(Marshal.GetLastWin32Error());
Console.WriteLine(Marshal.GetLastWin32Error());
// when you exchange them -> same behaviour just turned around

I don't see any difference! Both behave the same except Marshal.GetLastWin32Error stores results from App->CLR->WinApi calls as well and GetLastError stores only results from App->WinApi calls.


Garbage Collection seems not to call any WinApi functions overwriting the last error code
  • GetLastError is thread-safe. SetLastError stores an error code for each thread calling it.
  • since when would GC run in my threads ?
Bitterblue
  • 13,162
  • 17
  • 86
  • 124
  • 5
    `GetLastError` works, possibly it works in all existing .NET Framework versions and implementations. So, your code is working, but this doesn't prove anything. .NET Framework developers are free to change .NET implementation by the way, that `GetLastError` will stop work one day. – Alex F Jul 31 '13 at 07:12
  • 4
    .NET developers only guarantee, that `GetLastWin32Error` works correctly. You want to use `GetlastError` hack, possibly it will always work, but this is still hack. So, the question is somewhat philosophical: can we use hacks, if it is not proved, that it is incorrect. – Alex F Jul 31 '13 at 07:17
  • @AlexFarber That would be a bad .NET Framework version. I can imagine that a lot of software is running with the simple GetLastError because the programmer haven't heard of Marshal.GetLastWin32Error or whatever. Making the update you mentioned would break all those software. – Bitterblue Jul 31 '13 at 07:20
  • Maybe this is not convincing example... Anyway. Call Win32 API with SetLastError=true. Call another API with SetLastError=false. Call GetLastWin32Error - it keeps last error from the first API call, by definition. – Alex F Jul 31 '13 at 07:26
  • Regarding good or bad .NET Framework version - again, this is philosophy. You didn't convince me (and nobody else, I hope) to use GetLastError. But anyone didn't convince you to use GetLastWin32Error. – Alex F Jul 31 '13 at 07:29
  • 1
    @AlexFarber See [this](http://stackoverflow.com/questions/17918266/winapi-getlasterror-vs-marshal-getlastwin32error?noredirect=1#comment26214284_17918729). I wanted to know exactly if I must or don't have to through away all my uses of GetLastError in the existing code. Right now there is just no prove that it is unsafe. – Bitterblue Jul 31 '13 at 08:20
  • 1
    GetLastError() is thread safe, information from Microsoft : "The last-error code is maintained on a per-thread basis. Multiple threads do not overwrite each other's last-error code". –  Apr 08 '20 at 12:17

3 Answers3

87

You must always use the Marshal.GetLastWin32Error. The main problem is the garbage collector. If it runs between the call of SetVolumeLabel and the call of GetLastError then you will receive the wrong value, because the GC has surely overwritten the last result.

Therefore you always need to specify the SetLastError=true in the DllImport-Attribute:

[DllImport("kernel32.dll", SetLastError=true)]
static extern bool SetVolumeLabel(string lpRootPathName, string lpVolumeName);

This ensures that the marhsallling stub calls immediately after the native function the "GetLastError" and stores it in the local thread.

And if you have specified this attribute then the call to Marshal.GetLastWin32Error will always have the correct value.

For more info see also "GetLastError and managed code" by Adam Nathan.

Also other function from .NET can change the windows "GetLastError". Here is an example which produces different results:

using System.IO;
using System.Runtime.InteropServices;

public class ForceFailure
{
  [DllImport("kernel32.dll")]
  public static extern uint GetLastError();

  [DllImport("kernel32.dll", SetLastError = true)]
  private static extern bool SetVolumeLabel(string lpRootPathName, string lpVolumeName);

  public static void Main()
  {
    if (SetVolumeLabel("XYZ:\\", "My Imaginary Drive "))
      System.Console.WriteLine("It worked???");
    else
    {
      System.Console.WriteLine(Marshal.GetLastWin32Error());
      try
      {
        using (new FileStream("sdsdafsdfsdfs sdsd ", FileMode.Open)) {}
      }
      catch
      {
      }
      System.Console.WriteLine(GetLastError());
    }
  }
}

Also it seems that this is depended on the CLR which you are using! If you compile this with .NET2, it will produce "2 / 0"; if you switch to .NET 4, it will output "2 / 2"...

So it is depended on the CLR version, but you should not trust the native GetLastError function; always use the Marshal.GetLastWin32Error.

Martin Schneider
  • 14,263
  • 7
  • 55
  • 58
Jochen Kalmbach
  • 3,549
  • 17
  • 18
  • I couldn't create a wrong value returned by `GetLastError`. See my edit and example code. – Bitterblue Jul 29 '13 at 08:51
  • 3
    Well, your example kinda proves you wrong. If you exchange them you get the same **unwanted** behavior from `Marshal.GetLastWin32Error`. But this is actually bad programming. Like when you overwrite a variable and expect it to have the old value still. From what I understand the dllimport thing is well designed in .NET and the use of `GetLastError` is save although people try to scare you off using it. – Bitterblue Jul 30 '13 at 06:26
  • your ans should be accepted as the correct ans. The GetLastError article was an eye opener :) Thanks – Aster Veigas Apr 23 '14 at 06:54
  • I agree with @Bitterblue that the example is wrong or misleading at least.. Presumably the new FileStream either produces a new error or resets it. It's expected that the result returned by both Marshal.GetLastWin32Error and GetLastError both return a "new" result at that time. – BatteryBackupUnit Feb 19 '16 at 10:21
  • Hi @BatteryBackupUnit: The function is only there, to point out, that WE do not know which Win32 API might be called between the unmanaged and the managed part of the code... we did not write the CLR, so we need to assume that there might be other Win32 API calls is some special situations (like GC or other scenarios) which we do not control. Therefor we need to use "SetLastError" in order to be sure that we get the correct error code. – Jochen Kalmbach Mar 29 '18 at 07:27
  • @JochenKalmbach Sure thing. However, your example does not prove that. If you'd wanted to prove that, you'd have to perform a `Marshal.GetLastWin32Error()` and `GetLastError()` without any other calls in between. Now, if the two calls would return a different result you'd have proven that there's a difference. With your example the difference might just be that `new FileStream(...)` is performing a Win32 call. It might even be a `DllImport` with `SetLastError = true`! – BatteryBackupUnit Mar 29 '18 at 09:09
  • Anyone working with PInvoke should read both this question and answer in full. Not setting 'SetLastError' to true causes all kinds of grief. – Casey Jun 20 '18 at 17:10
  • *"The main problem is the garbage collector."* - I doubt this is accurate. The garbage collector runs on its own, dedicated thread, and the last error code is recorded per thread. That is, the garbage collector will not have an effect on the last error code stored on any client thread. – IInspectable Jun 16 '22 at 07:00
  • First: There are two kind of garbage collectors (server and workstation). Second: The dot net runtime also envolved over the last couple of years... so they also might improve/solve the problems they had ;) – Jochen Kalmbach Jun 17 '22 at 05:36
17

TL;DR

  • Do use [DllImport(SetLastError = true)] and Marshal.GetLastWin32Error()
  • perform the Marshal.GetLastWin32Error() immediately after a failing Win32 call and on the same thread.

Argumentation

As i read it, the official explanation why you need Marshal.GetLastWin32Error can be found here:

The common language runtime can make internal calls to APIs that overwrite the GetLastError maintained by the operating system.

To say it in other words:

Between your Win32 call which sets the error, the CLR may "insert" other Win32 calls which could overwrite the error. Specifying [DllImport(SetLastError = true)] makes sure that the CLR retrieves the error code before the CLR executes any unexpected Win32 calls. To access that variable we need to use Marshal.GetLastWin32Error.

Now what @Bitterblue found is that these "inserted calls" don't happen often - he couldn't find any. But that's not really surpising. Why? Because it's extremely difficult to "black box test" whether GetLastError works reliably:

  • you can detect unreliability only if a CLR-inserted Win32 call actually fails in the meantime.
  • failure of these calls may be dependent on internal/external factors. Such as time/timing, memory pressure, devices, state of computer, windows version...
  • insertion of Win32 calls by CLR may be dependent on external factors. So under some circumstances the CLR inserts a Win32 call, under others it doesn't.
  • behavior can change with different CLR versions as well

There's is one specific component - the Garbage collector (GC) - which is known to interrupt a .net thread if there's memory pressure and do some processing on that thread (see What happens during a garbage collection). Now if the GC were to execute a failing Win32 call, this would break your call to GetLastError.

To sum it up, you have a plethora of unknown factors which can influence the reliability of GetLastError. You'll most likely not find an unreliability problem when developing/testing, but it might blow up in production at any time. So do use [DllImport(SetLastError = true)] and Marshal.GetLastWin32Error() and improve your sleep quality ;-)

BatteryBackupUnit
  • 12,934
  • 1
  • 42
  • 68
5

in [DllImport("kernel32.dll", SetLastError = true)] does the SetLastError attribute make the Framework store the error code for the use of Marshal.GetLastWin32Error() ?

Yes, as is documented in DllImportAttribute.SetLastError Field

is there an example where plain GetLastError fails to give the correct result ?

As documented in Marshal.GetLastWin32Error Method, if the framework itself (e.g. the garbage collector) calls any native method that sets an error value between your calls to the native method and GetLastError you would get the error value of the framework's call instead of your call.

do I really HAVE to use Marshal.GetLastWin32Error() ?

Since you can't ensure that the framework will never call a native method between your call and the call to GetLastError, yes. Also, why not?

is this "problem" Framework version related ?

It could definitely be (e.g. changes in the garbage collector), but it doesn't have to.

cremor
  • 6,669
  • 1
  • 29
  • 72
  • I checked GC but can't see any problem yet. See edit in my example code. – Bitterblue Jul 29 '13 at 08:35
  • @mini-me GC was just an example, I have no idea if it actually could cause problems. But it can for sure run in your thread if concurrent GC is disabled. Don't only think about automatic and/or background operations in the framework. Maybe you will one time call some (managed) framework API that internally calls a native API between your calls to the native API and `GetLastError`. Do you really want to analyze the implementation of all calls you do before calling `GetLastError`? – cremor Jul 29 '13 at 10:22
  • No, but the first thing to do after calling a WinApi function is to call GetLastError not to call another WinApi function or play a round of Tetris. Besides `Marshal.GetLastWin32Error` is just the same value on a different layer. If the CLR is bad designed and calls other WInApi functions who can guarantee that it is not overwriting this value by another call of a `SetLastError = true` flagged import ? – Bitterblue Jul 29 '13 at 11:18
  • 2
    The CLR does not have, as a design goal, not calling other Windows API functions between two of your P/Invoke calls. Thus, if it does this under any circumstances, it is not an indication of bad design. There are a number of situations where the additional handling required to support managed code can result in extra processing behind the scenes in between two of your statements. This *is* by design, and the fact that it does it is not a flaw but actually allows your code to work the way you expect. `Marshal.GetLastWin32Error` was added specifically to make it still possible to work with APIs. – Jonathan Gilbert Aug 19 '15 at 16:32
  • 3
    Note that .NET has a guarantee that none of that hidden processing will overwrite `Marshal.GetLastWin32Error`'s stashed value. Thus, if you make a call to a P/Invoke function with `SetLastError = true`, and you do no other P/Invoke calls on the same thread (which also means not calling library functions that might themselves P/Invoke), then `Marshal.GetLastWin32Error` will return the value of `GetLastError` as it was at the moment of your original P/Invoke call's return. The managed world can control when `Marshal.GetLastWin32Error` changes, but not when the `GetLastError` API function does. – Jonathan Gilbert Aug 19 '15 at 16:38