none
Safe handle has been closed. RRS feed

  • Question

  • I have an application that needs impoersonation. Here is how I go about it:

    private static WindowsIdentity GetWindowsIdentity(string userName, string domain, string password)
    {
       
    IntPtr userHandle = IntPtr.Zero;
       
    bool loggedOn = Win32.LogonUser(userName, domain, password,
                                                      (
    int)Win32.LogonType.LOGON32_LOGON_INTERACTIVE,
                                                      (
    int)Win32.LogonProvider.LOGON32_PROVIDER_DEFAULT,
                                                     
    out userHandle);
       
    if (!loggedOn)
           
    throw new Win32Exception(Marshal.GetLastWin32Error());
       
    return new WindowsIdentity(userHandle);
    }

    . . . .

    WindowsIdentity
    identity = GetWindowsIdentity(User, Domain, Password);
    WindowsImpersonationContext impersonationContext = identity.Impersonate();
    try
    {
        . . . .
    }
    finally
    {
       
    if (impersonationContext != null)
        {
            impersonationContext.Undo();
        }
       
    if (identity != null)
        {
            identity.Dispose();
        }
    }

    The problem is that a few seconds after a batch of routines that use this impersonation have complated I get an ObjectDisposed exception with the message "Safe handle has been closed". Besides calling Undo on the impersonation context and disposing the identity what else do I need to do to avoid these exceptions? The stack strace looks like:

    "   at System.Security.Principal.Win32.ImpersonateLoggedOnUser(SafeTokenHandle hToken)\r\n   at System.Security.Principal.WindowsIdentity.SafeImpersonate(SafeTokenHandle userToken, WindowsIdentity wi, StackCrawlMark& stackMark)\r\n   at System.Security.Principal.WindowsIdentity.Impersonate(StackCrawlMark& stackMark)\r\n   at System.Security.SecurityContext.SetSecurityContext(SecurityContext sc, SecurityContext prevSecurityContext, StackCrawlMark& stackMark)\r\n   at System.Threading.ExecutionContext.SetExecutionContext(ExecutionContext executionContext)\r\n   at System.Threading.ExecutionContext.runTryCode(Object userData)\r\n   at System.Runtime.CompilerServices.RuntimeHelpers.ExecuteCodeWithGuaranteedCleanup(TryCode code, CleanupCode backoutCode, Object userData)\r\n   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)\r\n   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)\r\n   at System.Threading._TimerCallback.PerformTimerCallback(Object state)"

    Thank  you.

    Kevin

    Saturday, June 14, 2008 10:20 PM

Answers

  • First off, you are leaking a handle.  You must P/Invoke CloseHandle(userHandle) in your GetWindowsIdentity() method.

    The exception is fairly odd, it is consistent with the identity object getting disposed or garbage collected but the Impersonate() call not getting undone.  However, your call stack looks like one of a threadpool thread, invoked by a System.Threading.Timer callback.  You might have a race condition here, the timer callback using the impersonated identity just as that identity is getting disposed.  If that's the case, you have a tricky problem.  You'd want to impersonate only on the current thread, not all the threads.  I don't think Windows supports that.  Not much you can do but interlock the impersonation with any other threads that might get started while impersonation is in effect.  Explicitly *not* disposing the identity could work too but I'm not sure if that's 100% safe.  Just 99.999%.

    Hans Passant.
    • Proposed as answer by KevinBurton Monday, June 16, 2008 5:23 AM
    • Marked as answer by KevinBurton Monday, June 16, 2008 5:37 PM
    Sunday, June 15, 2008 12:25 PM
    Moderator
  • The WindowsIdentity constructor already dups the handle for you.  You can safely close it afterwards.  
    Hans Passant.
    • Marked as answer by KevinBurton Monday, June 16, 2008 5:37 PM
    Monday, June 16, 2008 9:04 AM
    Moderator

All replies

  • First off, you are leaking a handle.  You must P/Invoke CloseHandle(userHandle) in your GetWindowsIdentity() method.

    The exception is fairly odd, it is consistent with the identity object getting disposed or garbage collected but the Impersonate() call not getting undone.  However, your call stack looks like one of a threadpool thread, invoked by a System.Threading.Timer callback.  You might have a race condition here, the timer callback using the impersonated identity just as that identity is getting disposed.  If that's the case, you have a tricky problem.  You'd want to impersonate only on the current thread, not all the threads.  I don't think Windows supports that.  Not much you can do but interlock the impersonation with any other threads that might get started while impersonation is in effect.  Explicitly *not* disposing the identity could work too but I'm not sure if that's 100% safe.  Just 99.999%.

    Hans Passant.
    • Proposed as answer by KevinBurton Monday, June 16, 2008 5:23 AM
    • Marked as answer by KevinBurton Monday, June 16, 2008 5:37 PM
    Sunday, June 15, 2008 12:25 PM
    Moderator
  • Thank you for your response. Is there some way that I can ""dup" the handle? I am assuming that passing in a closed handle to the WindowsIdentity constructor would not be a good idea. Also If I explicitly Dispose of the WindowsIdentity why doesn't it take care of closing the handle?

    Thanks again.

     

    Kevin

    Monday, June 16, 2008 5:22 AM
  • The WindowsIdentity constructor already dups the handle for you.  You can safely close it afterwards.  
    Hans Passant.
    • Marked as answer by KevinBurton Monday, June 16, 2008 5:37 PM
    Monday, June 16, 2008 9:04 AM
    Moderator
  •  LogonUser does not reliably return a primary token.  You must call DuplicateToken to construct a primary token from the token returned by LogonUser.  The duplicated token (primary token) must be passed to the WindowsIdentity constructor.

    Craig
    Thursday, July 10, 2008 1:34 PM