From bbb7c7a0600b6f3489189d064d4b56755ad71c6f Mon Sep 17 00:00:00 2001 From: Kevin Thompson Date: Sun, 16 Jun 2019 16:57:23 -0400 Subject: [PATCH] Improved the impersonation revert process. The Dispose(bool) method will still act even if finalizing, but it armors its access to the server to ensure that it doesn't accidentally cause a nullref trying to revert itself. --- NSspi/Contexts/ImpersonationHandle.cs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/NSspi/Contexts/ImpersonationHandle.cs b/NSspi/Contexts/ImpersonationHandle.cs index 8bdbdf0..6c5c530 100644 --- a/NSspi/Contexts/ImpersonationHandle.cs +++ b/NSspi/Contexts/ImpersonationHandle.cs @@ -31,13 +31,16 @@ namespace NSspi.Contexts this.disposed = false; } + /// + /// Finalizes the ImpersonationHandle by reverting the impersonation. + /// ~ImpersonationHandle() { Dispose( false ); } /// - /// Reverts the impersonation. + /// Reverts impersonation. /// public void Dispose() { @@ -45,11 +48,27 @@ namespace NSspi.Contexts GC.SuppressFinalize( this ); } - protected virtual void Dispose( bool disposing ) + /// + /// Reverts impersonation. + /// + /// True if being disposed, false if being finalized. + private void Dispose( bool disposing ) { - if( disposing && this.disposed == false && this.server != null && this.server.Disposed == false ) + // This implements a variant of the typical dispose pattern. Always try to revert + // impersonation, even if finalizing. Don't do anything if we're already reverted. + + if( this.disposed == false ) { - this.server.RevertImpersonate(); + this.disposed = true; + + // Just in case the reference is being pulled out from under us, pull a stable copy + // of the reference while we're null-checking. + var serverCopy = this.server; + + if( serverCopy != null && serverCopy.Disposed == false ) + { + serverCopy.RevertImpersonate(); + } } } }