Shouldn't GetLastWin32Error be reset if P/Invoke attribute SetLastError = true?

Question

I noticed that Marshal.GetLastWin32Error() was returning error 122 during the dispose method after a p/invoke to Kernal32.CloseHandle(IntPtr p_handle) even with the import attribute SetLastError set to true on the import definition. I tracked the cause down to the creation of new WindowsIdentity just before the handle being closed and I'm wondering what the proper way to handle this is.

1. Should I only check for a Win32 error if CloseHandle returns false?
2. Forcefully SetLastError to 0 before p/invoking CloseHandle?
3. Forcefully SetLastError to 0 after new WindowsIdentity(DangereousHandle)

Point 3 seems immediately unreasonable to me, so I presume that's an obvious no.

Point 2 seems most reasonable, but perhaps unnecessarily aggressive. The only reason I would go with this instead of point 1 is because I'm not sure I have a guarantee that if CloseHandle returns false the error code will be relevant. Is that guaranteed by the function documentation?

Point 1 seems like it's what I should be doing. That is I should only be calling GetLastWin32Error when CloseHandle explicitly returns false and assume that the returned code will indicate why CloseHandle failed and not some prior unrelated Win API call.

Code

using System;
using System.Runtime.ConstrainedExecution;
using System.Runtime.InteropServices;
using System.Security;

namespace Common.InterOp
{
    // CloseHandle http://msdn.microsoft.com/en-ca/library/windows/desktop/ms724211%28v=vs.85%29.aspx

    internal static partial class Kernel32
    {
        [DllImport("kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)]
        [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)]
        [SuppressUnmanagedCodeSecurity]
        [return: MarshalAs(UnmanagedType.Bool)]
        internal static extern bool CloseHandle(IntPtr handle);
    }
}

Here's where Close Handle is called

using Common.InterOp;
using Microsoft.Win32.SafeHandles;
using System.ComponentModel;
using System.Runtime.InteropServices;

namespace Common.InterOp
{
    // http://msdn.microsoft.com/en-us/library/system.security.principal.windowsimpersonationcontext%28v=vs.100%29.aspx

    public sealed class SafeTokenHandle : SafeHandleZeroOrMinusOneIsInvalid
    {
        private SafeTokenHandle()
            : base(true)
        { }

        protected override bool ReleaseHandle()
        {
            int Win32Error =
                Marshal.GetLastWin32Error(); // <-- Already has code 122

            var Closed =
                Kernel32.CloseHandle(handle); // <-- has SetLastError=true attribute

            Win32Error =
                Marshal.GetLastWin32Error(); // <-- Still has code 122

            if (!Closed && Win32Error != 0) // This works, but is it correct?
                throw
                    new Win32Exception(Win32Error);

            return Closed;
        }
    }
}

Here's the code block that actually generates error 122 on the line that has var WinID = new WindowsIdentity(DangerousHandle);

using System;
using System.ComponentModel;
using System.Runtime.InteropServices;
using System.Security;
using System.Security.Principal;
using Common.InterOp;
using Common.Environment;

namespace Common.Authentication
{
    public static partial class Authentication
    {
        /// <summary>
        /// Authenticates the log on credentials of a Windows user.
        /// Returns a WindowsIdentity if authentication succeeds for the specified user; Otherwise returns null.
        /// </summary>
        /// <param name="p_userName">The user's Windows account name.</param>
        /// <param name="p_password">The user's Windows account password.</param>
        /// <param name="p_domainName">The user's domain name or the name of the local machine for non-networked accounts.</param>
        /// <returns>a <seealso cref="WindowsIdentity"/> if authentication succeeds for the specified user; Otherwise returns null.</returns>
        /// <exception cref="System.ArgumentException"></exception>
        /// <exception cref="System.ComponentModel.Win32Exception"></exception>
        /// <exception cref="System.Security.SecurityException"></exception>
        public static WindowsIdentity LogonUser(SecureString p_userName, SecureString p_password, SecureString p_domainName, LogonUserOptions p_logonOptions)
        {
            // The following functions seem related, but it's unclear if or in what scenario they would ever be required
            // DuplicateToken   http://msdn.microsoft.com/en-us/library/windows/desktop/aa446616%28v=vs.85%29.aspx 
            // DuplicateTokenEx http://msdn.microsoft.com/en-us/library/windows/desktop/aa446617%28v=vs.85%29.aspx

            SafeTokenHandle UserAccountToken = null;

            try
            {
                using (var DecryptedUserName = new DecryptAndMarshallSecureString(p_userName))
                using (var DecryptedPassword = new DecryptAndMarshallSecureString(p_password))
                using (var DecryptedDomainName = new DecryptAndMarshallSecureString(p_domainName))
                {
                    // Call LogonUser, passing the unmanaged (and decrypted) copies of the SecureString credentials.
                    bool ReturnValue =
                        AdvApi32.LogonUser(
                            p_userName: DecryptedUserName.GetHandle(),
                            p_domainName: DecryptedDomainName.GetHandle(),
                            p_password: DecryptedPassword.GetHandle(),
                            p_logonType: p_logonOptions.Type,           // LogonType.LOGON32_LOGON_INTERACTIVE,
                            p_logonProvider: p_logonOptions.Provider,   // LogonProvider.LOGON32_PROVIDER_DEFAULT,
                            p_userToken: out UserAccountToken);

                    // Get the Last win32 Error and throw an exception.
                    int Win32Error = Marshal.GetLastWin32Error();
                    if (!ReturnValue && UserAccountToken.DangerousGetHandle() == IntPtr.Zero)
                        throw
                            new Win32Exception(Win32Error);

                    // The token that is passed to the following constructor must  
                    // be a primary token in order to use it for impersonation.

                    var DangerousHandle = UserAccountToken.DangerousGetHandle();

                    Win32Error = Marshal.GetLastWin32Error(); // No error

                    var WinID = new WindowsIdentity(DangerousHandle);

                    Win32Error = Marshal.GetLastWin32Error(); // error 122

                    // Kernel32.SetLastError(0); // Resets error to 0
                    Sleep.For(0);
                    //new Win32Exception(Win32Error); // Also resets error to 0

                    Win32Error = Marshal.GetLastWin32Error(); // Still error 122 unless one of the reset lines above is uncommented

                    return WinID;
                }
            }
            finally
            {
                if (UserAccountToken != null)
                    UserAccountToken.Dispose(); // This calls CloseHandle which is where I first noticed the error being non-zero
            }
        }
    }
}

EDIT 2014, 08, 09

Updated class based on hvd's comments and answer.

using Common.Attributes;
using Microsoft.Win32.SafeHandles;
using System.ComponentModel;
using System.Runtime.ConstrainedExecution;

namespace Common.InterOp
{
    // SafeHandle.ReleaseHandle Method http://msdn.microsoft.com/en-us/library/system.runtime.interopservices.safehandle.releasehandle%28v=vs.110%29.aspx
    // http://msdn.microsoft.com/en-us/library/system.security.principal.windowsimpersonationcontext%28v=vs.100%29.aspx
    // http://stackoverflow.com/questions/25206969/shouldnt-getlastwin32error-be-reset-if-p-invoke-attribute-setlasterror-true

    [jwdebug(2014, 08, 09, "releaseHandleFailed MDA http://msdn.microsoft.com/en-us/library/85eak4a0%28v=vs.110%29.aspx")]
    public sealed class SafeTokenHandle : SafeHandleZeroOrMinusOneIsInvalid
    {
        private SafeTokenHandle()
            : base(true)
        { }

        [ReliabilityContract(Consistency.WillNotCorruptState, Cer.MayFail)]
        override protected bool ReleaseHandle()
        {
            // Here, we must obey all rules for constrained execution regions. 

            return
                Kernel32.CloseHandle(handle);

            // If ReleaseHandle failed, it can be reported via the 
            // "releaseHandleFailed" managed debugging assistant (MDA).  This
            // MDA is disabled by default, but can be enabled in a debugger 
            // or during testing to diagnose handle corruption problems. 
            // We do not throw an exception because most code could not recover 
            // from the problem.
        }
    }
}

Answer 1

Point 1 seems like it's what I should be doing.

Yes, that's exactly right. Forget the .NET P/Invoke aspect of your question, and look at the original function description:

Return value

If the function succeeds, the return value is nonzero.

If the function fails, the return value is zero. To get extended error information, call GetLastError.

The description doesn't say that calling GetLastError is meaningful when the function succeeds, and as you found out, it indeed isn't meaningful, so you shouldn't be calling it in that case.