Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e184e80
Fix SQL Server and certificate resource leaks in test suites
cheenamalhotra Aug 25, 2026
c7ec9ab
Address review feedback on cleanup guards
cheenamalhotra Aug 25, 2026
f1f7f4a
Harden DatabaseObject cleanup against ambiguous-completion and broken…
cheenamalhotra Aug 25, 2026
42d41f8
Move connection Open() inside constructor cleanup guards
cheenamalhotra Aug 25, 2026
e91695e
Annotate VectorBackwardCompatTestBase.DisposeSafely parameter as null…
cheenamalhotra Aug 25, 2026
b87a188
Consolidate BulkCopy tests onto shared DatabaseObject RAII types
cheenamalhotra Aug 25, 2026
969eddf
Convert AE fixtures onto shared DatabaseObject types
cheenamalhotra Aug 25, 2026
949ad07
Report orphaned objects when the final drop attempt fails
cheenamalhotra Aug 26, 2026
ea7ed97
Make DatabaseObject.Dispose best-effort so cleanup cannot mask a test…
cheenamalhotra Aug 26, 2026
e09d7d2
Remove dstCmd locals orphaned by the RAII table conversion
cheenamalhotra Aug 26, 2026
bbc3447
Dispose SqlCommands in ParallelTransactionsTest exception-safely
cheenamalhotra Aug 26, 2026
2c6fd9b
Dispose commands and transactions in the BulkCopy transaction tests
cheenamalhotra Aug 26, 2026
9ab0256
Report certificate store cleanup failures instead of swallowing them
cheenamalhotra Aug 26, 2026
47a6eef
Make the two remaining finally-block drops best-effort
cheenamalhotra Aug 26, 2026
8db1e6d
Simplify DatabaseObject RAII contract per review
cheenamalhotra Aug 26, 2026
36a765c
Add TemporalTable primitive; restore object adoption
cheenamalhotra Aug 26, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,38 @@ public void Dispose()

protected virtual void Dispose(bool disposing)
{
foreach (KeyVaultKey key in _createdKeys)
try
{
try
{
_keyClient.StartDeleteKey(key.Name).WaitForCompletion();
}
catch (Exception)
foreach (KeyVaultKey key in _createdKeys)
{
continue;
try
{
_keyClient.StartDeleteKey(key.Name).WaitForCompletion();
}
catch (Exception)
{
continue;
}

// A deleted key remains in a soft-deleted state (and continues to consume the name)
// until it is purged or the retention period expires. Purging is best-effort: the
// test principal may not have permission, and the vault may have purge protection
// enabled, in which case the soft-deleted key simply expires on its own.
try
{
_keyClient.PurgeDeletedKey(key.Name);
}
catch (Exception)
{
continue;
}
}
}

_randomGenerator.Dispose();
_createdKeys.Clear();
}
finally
{
_randomGenerator.Dispose();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,23 @@ public CertificateStoreContext(StoreLocation location, StoreName name)

private readonly List<CertificateStoreContext> _certificateStoreModifications = new List<CertificateStoreContext>();

/// <summary>
/// Every certificate handed out by <see cref="CreateCertificate"/>. Certificates are created with
/// <see cref="X509KeyStorageFlags.PersistKeySet"/>, which writes the private key to a key container on
/// disk. Removing the certificate from a store does not remove that container, so the containers must
/// be deleted explicitly or they accumulate indefinitely across test runs.
/// </summary>
private readonly List<X509Certificate2> _createdCertificates = new List<X509Certificate2>();

protected X509Certificate2 CreateCertificate(string subjectName, IEnumerable<string> dnsNames, IEnumerable<string> ipAddresses, bool forceCsp = false)
{
X509Certificate2 certificate = CreateCertificateCore(subjectName, dnsNames, ipAddresses, forceCsp);

_createdCertificates.Add(certificate);
return certificate;
}

private X509Certificate2 CreateCertificateCore(string subjectName, IEnumerable<string> dnsNames, IEnumerable<string> ipAddresses, bool forceCsp = false)
{
// This will always generate a certificate with:
// * Start date: 24hrs ago
Expand Down Expand Up @@ -281,37 +297,121 @@ public void Dispose()

protected virtual void Dispose(bool disposing)
{
// Collect everything that needs disposing before touching any of it: removal from a store and
// deletion of the persisted key are both best-effort, but the handles must always be released.
List<X509Certificate2> certificates = new List<X509Certificate2>(_createdCertificates);

// Remove the certificates from any store they were added to. This must happen before the
// certificates are disposed, because a disposed certificate cannot be matched against a store.
foreach (CertificateStoreContext storeContext in _certificateStoreModifications)
{
using X509Store store = new X509Store(storeContext.Name, storeContext.Location);

bool opened;
try
{
store.Open(OpenFlags.ReadWrite);
opened = true;
}
catch (Exception)
catch (Exception ex)
{
continue;
// Every certificate destined for this store is about to be skipped, so this is the
// only chance to say so. Without it the certificates leak across runs with no trace.
Console.WriteLine(
$"Failed to open certificate store '{storeContext.Name}' in '{storeContext.Location}' " +
$"for cleanup; {storeContext.Certificates.Count} certificate(s) may be left behind. {ex}");

opened = false;
}

foreach (X509Certificate2 cert in storeContext.Certificates)
{
if (!certificates.Contains(cert))
{
certificates.Add(cert);
}

if (!opened)
{
continue;
}

try
{
if (store.Certificates.Contains(cert))
{
store.Remove(cert);
}
}
catch (Exception)
catch (Exception ex)
{
// Same reasoning as the store-open failure above: report the leak rather than
// letting the certificate linger in the store silently.
Console.WriteLine(
$"Failed to remove certificate '{cert.Subject}' from store '{storeContext.Name}' " +
$"in '{storeContext.Location}'; it may be left behind. {ex}");

continue;
}
}

storeContext.Certificates.Clear();
}

_certificateStoreModifications.Clear();

foreach (X509Certificate2 cert in certificates)
{
DeletePersistedPrivateKey(cert);

try
{
cert.Dispose();
}
catch (Exception)
{
// Nothing further can be done about a certificate that refuses to release its handle.
}
}

storeContext.Certificates.Clear();
_createdCertificates.Clear();
}

/// <summary>
/// Deletes the on-disk key container backing a certificate's private key, if there is one.
/// </summary>
/// <remarks>
/// This is best-effort: the certificate may have no private key, the key may be ephemeral, or the
/// current user may lack permission to delete it (for example, for a machine-scoped key).
/// </remarks>
private static void DeletePersistedPrivateKey(X509Certificate2 certificate)
{
try
{
if (!certificate.HasPrivateKey)
{
return;
}

using RSA? privateKey = certificate.GetRSAPrivateKey();

switch (privateKey)
{
case RSACryptoServiceProvider csp:
csp.PersistKeyInCsp = false;
break;
#if NET
case RSACng cng:
cng.Key.Delete();
break;
#endif
default:
break;
}
}
catch (Exception)
{
// Best-effort; a stale key container is preferable to failing the test run during cleanup.
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,41 @@ public sealed class ColumnEncryptionCertificateFixture : CertificateFixtureBase

public ColumnEncryptionCertificateFixture()
{
PrimaryColumnEncryptionCertificate = CreateCertificate(nameof(PrimaryColumnEncryptionCertificate), Array.Empty<string>(), Array.Empty<string>());
SecondaryColumnEncryptionCertificate = CreateCertificate(nameof(SecondaryColumnEncryptionCertificate), Array.Empty<string>(), Array.Empty<string>());
_currentUserCertificate = CreateCertificate(nameof(_currentUserCertificate), Array.Empty<string>(), Array.Empty<string>());
using (X509Certificate2 createdCertificate = CreateCertificate(nameof(CertificateWithoutPrivateKey), Array.Empty<string>(), Array.Empty<string>()))
// NOTE: If this constructor throws, xUnit never calls Dispose, so any certificate already
// placed in a store (and its persisted key container) would be leaked.
try
{
PrimaryColumnEncryptionCertificate = CreateCertificate(nameof(PrimaryColumnEncryptionCertificate), Array.Empty<string>(), Array.Empty<string>());
SecondaryColumnEncryptionCertificate = CreateCertificate(nameof(SecondaryColumnEncryptionCertificate), Array.Empty<string>(), Array.Empty<string>());
_currentUserCertificate = CreateCertificate(nameof(_currentUserCertificate), Array.Empty<string>(), Array.Empty<string>());

// NOTE: The source certificate is intentionally not disposed here; it is tracked by the base
// fixture, which deletes its persisted key container and disposes it on cleanup.
X509Certificate2 createdCertificate = CreateCertificate(nameof(CertificateWithoutPrivateKey), Array.Empty<string>(), Array.Empty<string>());

// This will strip the private key away from the created certificate
#if NET9_0_OR_GREATER
CertificateWithoutPrivateKey = X509CertificateLoader.LoadCertificate(createdCertificate.Export(X509ContentType.Cert));
CertificateWithoutPrivateKey = X509CertificateLoader.LoadCertificate(createdCertificate.Export(X509ContentType.Cert));
#else
CertificateWithoutPrivateKey = new X509Certificate2(createdCertificate.Export(X509ContentType.Cert));
#endif
AddToStore(CertificateWithoutPrivateKey, StoreLocation.CurrentUser, StoreName.My);
}

AddToStore(PrimaryColumnEncryptionCertificate, StoreLocation.CurrentUser, StoreName.My);
AddToStore(SecondaryColumnEncryptionCertificate, StoreLocation.CurrentUser, StoreName.My);
AddToStore(_currentUserCertificate, StoreLocation.CurrentUser, StoreName.My);
AddToStore(PrimaryColumnEncryptionCertificate, StoreLocation.CurrentUser, StoreName.My);
AddToStore(SecondaryColumnEncryptionCertificate, StoreLocation.CurrentUser, StoreName.My);
AddToStore(_currentUserCertificate, StoreLocation.CurrentUser, StoreName.My);

if (IsAdmin)
{
_localMachineCertificate = CreateCertificate(nameof(_localMachineCertificate), Array.Empty<string>(), Array.Empty<string>());
if (IsAdmin)
{
_localMachineCertificate = CreateCertificate(nameof(_localMachineCertificate), Array.Empty<string>(), Array.Empty<string>());

AddToStore(_localMachineCertificate, StoreLocation.LocalMachine, StoreName.My);
AddToStore(_localMachineCertificate, StoreLocation.LocalMachine, StoreName.My);
}
}
catch
{
Dispose();
throw;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,21 @@ protected ColumnMasterKeyCertificateFixture(bool createCertificate)
{
if (createCertificate)
{
ColumnMasterKeyCertificate = CreateCertificate(nameof(ColumnMasterKeyCertificate), Array.Empty<string>(), Array.Empty<string>());

AddToStore(ColumnMasterKeyCertificate, StoreLocation.CurrentUser, StoreName.My);

ColumnMasterKeyCertificatePath = $"{StoreLocation.CurrentUser}/{StoreName.My}/{ColumnMasterKeyCertificate.Thumbprint}";
// NOTE: If this constructor throws, xUnit never calls Dispose, so the certificate placed in
// the store (and its persisted key container) would be leaked.
try
{
ColumnMasterKeyCertificate = CreateCertificate(nameof(ColumnMasterKeyCertificate), Array.Empty<string>(), Array.Empty<string>());

AddToStore(ColumnMasterKeyCertificate, StoreLocation.CurrentUser, StoreName.My);

ColumnMasterKeyCertificatePath = $"{StoreLocation.CurrentUser}/{StoreName.My}/{ColumnMasterKeyCertificate.Thumbprint}";
}
catch
{
Dispose();
throw;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,22 @@ public class CspCertificateFixture : CertificateFixtureBase
{
public CspCertificateFixture()
{
CspCertificate = CreateCertificate(nameof(CspCertificate), Array.Empty<string>(), Array.Empty<string>(), true);
// NOTE: If this constructor throws, xUnit never calls Dispose, so the certificate placed in the
// store (and its persisted CSP key container) would be leaked.
try
{
CspCertificate = CreateCertificate(nameof(CspCertificate), Array.Empty<string>(), Array.Empty<string>(), true);

AddToStore(CspCertificate, StoreLocation.CurrentUser, StoreName.My);
AddToStore(CspCertificate, StoreLocation.CurrentUser, StoreName.My);

CspCertificatePath = $"{StoreLocation.CurrentUser}/{StoreName.My}/{CspCertificate.Thumbprint}";
CspKeyPath = GetCspPathFromCertificate();
CspCertificatePath = $"{StoreLocation.CurrentUser}/{StoreName.My}/{CspCertificate.Thumbprint}";
CspKeyPath = GetCspPathFromCertificate();
}
catch
{
Dispose();
throw;
}
}

public X509Certificate2 CspCertificate { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public sealed class ColumnEncryptionKey : DatabaseObject<ColumnMasterKey>
/// <param name="cmkOrigin">The column master key which backs this encryption key.</param>
public ColumnEncryptionKey(SqlConnection connection, string namePrefix, ColumnMasterKey cmkOrigin)
: base(connection, GenerateLongName(namePrefix), definition: DefinitionTemplate,
state: cmkOrigin, shouldCreate: true, shouldDrop: true)
state: cmkOrigin)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal CreationParameters(SqlColumnEncryptionKeyStoreProvider provider,

protected ColumnMasterKey(SqlConnection connection, string namePrefix, CreationParameters creationParameters)
: base(connection, name: GenerateLongName(namePrefix), definition: DefinitionTemplate,
state: creationParameters, shouldCreate: true, shouldDrop: true)
state: creationParameters)
{
}

Expand Down
Loading
Loading