Improve the reliability of the SecureBufferAdapter, so that we can make stronger guarantees that the held GC handles will be released even if the object is leaked.
Also, I added guarding in the Dispose(false) method so that we don't accidentally try to release a handle that was never allocated, possibly causing us to throw an exception and fail to release the handles further down the execution.
This commit is contained in:
@@ -1,13 +1,14 @@
|
|||||||
using System;
|
using System;
|
||||||
using System.Collections.Generic;
|
using System.Collections.Generic;
|
||||||
using System.Linq;
|
using System.Linq;
|
||||||
|
using System.Runtime.ConstrainedExecution;
|
||||||
using System.Runtime.InteropServices;
|
using System.Runtime.InteropServices;
|
||||||
using System.Text;
|
using System.Text;
|
||||||
using System.Threading.Tasks;
|
using System.Threading.Tasks;
|
||||||
|
|
||||||
namespace NSspi.Buffers
|
namespace NSspi.Buffers
|
||||||
{
|
{
|
||||||
internal class SecureBufferAdapter : IDisposable
|
internal sealed class SecureBufferAdapter : CriticalFinalizerObject, IDisposable
|
||||||
{
|
{
|
||||||
private bool disposed;
|
private bool disposed;
|
||||||
|
|
||||||
@@ -27,8 +28,7 @@ namespace NSspi.Buffers
|
|||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
//[ReliabilityContract( Consistency.MayCorruptAppDomain, Cer.None)]
|
public SecureBufferAdapter( IList<SecureBuffer> buffers ) : base()
|
||||||
public SecureBufferAdapter( IList<SecureBuffer> buffers )
|
|
||||||
{
|
{
|
||||||
this.buffers = buffers;
|
this.buffers = buffers;
|
||||||
|
|
||||||
@@ -57,8 +57,12 @@ namespace NSspi.Buffers
|
|||||||
this.descriptorHandle = GCHandle.Alloc( descriptor, GCHandleType.Pinned );
|
this.descriptorHandle = GCHandle.Alloc( descriptor, GCHandleType.Pinned );
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[ReliabilityContract( Consistency.WillNotCorruptState, Cer.Success )]
|
||||||
~SecureBufferAdapter()
|
~SecureBufferAdapter()
|
||||||
{
|
{
|
||||||
|
// We bend the typical Dispose pattern here. This finalizer runs in a Constrained Execution Region,
|
||||||
|
// and so we shouldn't call virtual methods. There's no need to extend this class, so we prevent it
|
||||||
|
// and mark the protected Dispose method as non-virtual.
|
||||||
Dispose( false );
|
Dispose( false );
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -81,25 +85,36 @@ namespace NSspi.Buffers
|
|||||||
GC.SuppressFinalize( this );
|
GC.SuppressFinalize( this );
|
||||||
}
|
}
|
||||||
|
|
||||||
protected virtual void Dispose( bool disposing )
|
[ReliabilityContract( Consistency.WillNotCorruptState, Cer.Success )]
|
||||||
|
private void Dispose( bool disposing )
|
||||||
{
|
{
|
||||||
if ( this.disposed == true ) { return; }
|
if ( this.disposed == true ) { return; }
|
||||||
|
|
||||||
if ( disposing )
|
if ( disposing )
|
||||||
{
|
{
|
||||||
for ( int i = 0; i < this.buffers.Count; i++ )
|
for( int i = 0; i < this.buffers.Count; i++ )
|
||||||
{
|
{
|
||||||
this.buffers[i].Length = this.bufferCarrier[i].Count;
|
this.buffers[i].Length = this.bufferCarrier[i].Count;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
for ( int i = 0; i < this.bufferHandles.Length; i++ )
|
for( int i = 0; i < this.bufferHandles.Length; i++ )
|
||||||
{
|
{
|
||||||
this.bufferHandles[i].Free();
|
if( this.bufferHandles[i].IsAllocated )
|
||||||
|
{
|
||||||
|
this.bufferHandles[i].Free();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
this.bufferCarrierHandle.Free();
|
if( this.bufferCarrierHandle.IsAllocated )
|
||||||
this.descriptorHandle.Free();
|
{
|
||||||
|
this.bufferCarrierHandle.Free();
|
||||||
|
}
|
||||||
|
|
||||||
|
if( this.descriptorHandle.IsAllocated )
|
||||||
|
{
|
||||||
|
this.descriptorHandle.Free();
|
||||||
|
}
|
||||||
|
|
||||||
this.disposed = true;
|
this.disposed = true;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user