Dispose() should only free unmanaged memory when called from finalizer thread

[jgilbert2017]

per MS docs it's probably a no-no to touch managed memory in Dispose() when it is called from a finalizer.

from a global code search this rule does not seem to be followed.

for example, CopyStream.cs contains

        protected virtual void Dispose(bool disposing)
        {
            if (disposed)
            {
                return;
            }

            disposed = true;
            bufferPool.ReturnBuffer(buffer);
        }

which tries to return the buffer to the pool. my guess is that this is unsafe to do from a finalizer.

https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose "If the method call comes from a finalizer, only the code that frees unmanaged resources should execute. The implementer is responsible for ensuring that the false path doesn't interact with managed objects that may have been disposed. This is important because the order in which the garbage collector disposes managed objects during finalization is non-deterministic."

some additional explanation https://stackoverflow.com/questions/31703447/why-should-dispose-dispose-managed-resources-and-finalizer-not

[honfika]

fixed: https://github.com/justcoding121/titanium-web-proxy/commit/9d2b5340bf834773679f381fc1273531bdd1c027

[jgilbert2017]

the code in CopyStream still seems to be there. i don't think it is valid to return the buffer to the pool from a finalizer. there are also many classes where work is being done from the finalizer (eg CertificateManager, ProxyServer, SessionEventArgsBase, TcpClientConnection, TcpConnectionFactory, TcpServerConnection, WinHttpWebProxyFinder) which I don't think is valid/a good idea.

[honfika]

Checked all the mentioned classes: https://github.com/justcoding121/titanium-web-proxy/commit/8d6c23fcd30ec6d0450a9b599c69c9f9879090ee

But anyway, the finalizers should not be called in those classes. (Except the ProxyServer class, which will be finalized when the user does not call the Dispose)

[jgilbert2017]

it's looking better however the following code paths are still active via the finalizer: ProxyServer.Dispose(bool) calls Stop() TcpClientConnection.Dispose(bool) calls Task.Run() TcpServerConnection.Dispose(bool) calls Task.Run() SessionEventArgs/SessionArgsBase/TunnelConnetSessionEventArgs null out some event handlers. probably ok but i'm not 100% sure it is safe.