Not disposing X509Certificates leads to native memory leak

[koepalex]

Type of issue

• Bug
• Enhancement
• Compliance
• Question
• Help wanted

Current Behavior

CertificateValidator has strong reference to various X509Certificates (m_applicationCertificate, m_validatedCertificates) and other disposable members (m_semaphore, m_issuerCertificateList, m_trustedCertificateList) but does not implement IDisposable to clean them up.

This can lead to native memory leak, when the finalizer can't garbage collect the X509Certificates (SafeHandles pointing to native memory containing the certificate) fast enough.

Expected Behavior

(I)CertificateValidator implements IDisposable:

        /// <summary>
        /// Releases the resources used by the current instance of the class.
        /// </summary>
        /// <remarks>This method should be called when the instance is no longer needed to free up
        /// resources.  It suppresses finalization to prevent the garbage collector from calling the
        /// finalizer.</remarks>
        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }

        /// <summary>
        /// Releases the resources used by the current instance of the class.
        /// </summary>
        /// <remarks>This method is called by the public <see cref="Dispose()"/> method </remarks>
        protected virtual void Dispose(bool disposing)
        {
            if (disposing)
            {
                Utils.SilentDispose(m_applicationCertificate);
                Utils.SilentDispose(m_semaphore);
                foreach (var cert in m_validatedCertificates.Values)
                {
                    Utils.SilentDispose(cert);
                }
                Utils.SilentDispose(m_issuerCertificateList);
                Utils.SilentDispose(m_trustedCertificateList);
                m_CertificateValidation = null;
                m_CertificateUpdate = null;
            }
        }

ApplicationConfiguration implements IDisposable (to clean Certificate Validator):

        /// <summary>
        /// Releases the resources used by the current instance of the class.
        /// </summary>
        /// <remarks>This method should be called when the instance is no longer needed to free up
        /// resources.  It suppresses finalization to prevent the garbage collector from calling the
        /// finalizer.</remarks>
        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }

        /// <summary>
        /// Releases the resources used by the current instance of the class.
        /// </summary>
        /// <remarks>This method is called by the public <see cref="Dispose()"/> method </remarks>
        protected virtual void Dispose(bool disposing)
        {
            if (disposing)
            {
                Utils.SilentDispose(m_certificateValidator);
            }
        }

The Session than should clean the ApplicationConfiguration:

protected override void Dispose(bool disposing)
{
    if (disposing)
    {
        StopKeepAliveTimer();

        Utils.SilentDispose(m_defaultSubscription);
        m_defaultSubscription = null;

        Utils.SilentDispose(m_nodeCache);
        m_nodeCache = null;

        List<Subscription> subscriptions = null;
        lock (SyncRoot)
        {
            subscriptions = new List<Subscription>(m_subscriptions);
            m_subscriptions.Clear();
        }

        foreach (Subscription subscription in subscriptions)
        {
            Utils.SilentDispose(subscription);
        }
        subscriptions.Clear();

        Utils.SilentDispose(m_reconnectLock);
        Utils.SilentDispose(m_configuration);
    }

    base.Dispose(disposing);

    if (disposing)
    {
        // suppress spurious events
        m_KeepAlive = null;
        m_Publish = null;
        m_PublishError = null;
        m_PublishSequenceNumbersToAcknowledge = null;
        m_SubscriptionsChanged = null;
        m_SessionClosing = null;
        m_SessionConfigurationChanged = null;
    }
}

Steps To Reproduce

1. Create a session to an OPC UA server
2. Close the session e.g. not gracefully by trying to monitor an OPC UA datapoint by expanded node id, where the namespaces does not exist in namespace table
3. wait some backoff time (e.g. 30s)
4. repeat from step 1

The memory will constantly be increasing and the number of SafeHandles in memory will be growing. Can be checked by running dumpheap -stat -type Microsoft.Win32.SafeHandles.SafeX509Handle via dotnet dump or windbg on a memory dump.

Environment

- OS: any
- Environment: 
- Runtime: dotnet9/dotnet10
- Nuget Version: 1.5.377.21
- Component: Opc.Ua.Core
- Server: n/a
- Client: own client

Anything else?

Implementing the change suggested causing a lot of tests e.g. Opc.Ua.Client.Tests when running together to fail, as they will partially reuse Session/ApplicationContext objects.