Fix for ObjectDisposedException in PodeRequest.Receive During SSL/TLS Operations

[mdaneri]

Pull Request: Fix for ObjectDisposedException in PodeRequest.Receive During SSL/TLS Operations

Summary

This pull request addresses #1459
Intermittent ObjectDisposedException that occurs in the PodeRequest.Receive method during SSL/TLS operations in Pode. The issue arises due to premature disposal or concurrent access of the SslStream instance when handling HTTPS requests under high concurrency.

Problem

The following error was observed when running a Pode server with an HTTPS endpoint under load:

[Error] ObjectDisposedException: Cannot access a disposed object.
Object name: 'SslStream'.
   at System.Net.Security.SslState.CheckThrow(Boolean authSuccessCheck, Boolean shutdownCheck)
   at System.Net.Security.SslState.get_SecureStream()
   at System.Net.Security.SslStream.EndRead(IAsyncResult asyncResult)
   at System.Threading.Tasks.TaskFactory`1.FromAsyncTrimPromise`1.Complete(TInstance thisRef, Func`3 endMethod, IAsyncResult asyncResult, Boolean requiresSynchronization)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Pode.PodeRequest.<Receive>d__83.MoveNext()

The issue is caused by:

1. Premature Disposal: The SslStream instance is disposed of while operations are still in progress.
2. Concurrent Access: Multiple threads attempting to access the SslStream without proper synchronization.

Changes Made

1. Improved Thread Safety:

   • Added additional checks to ensure that all operations on SslStream are thread-safe, using SemaphoreSlim to manage concurrency.

2. Enhanced Disposal Logic:

   • Updated the Dispose method in PodeRequest to ensure that the SslStream is not disposed prematurely.
   • Introduced IsDisposed and IsOpen state checks to prevent operations on already-disposed streams.

3. Error Logging:

   • Adjusted error logging levels for expected exceptions (ObjectDisposedException and IOException) to avoid alarming users unnecessarily. These exceptions are now logged at the Verbose level instead of Error.

4. Buffer Management:

   • Optimized buffer handling to prevent unnecessary reinitializations and ensure reuse during stream operations.

5. Stream Cleanup:

   • Ensured proper handling and disposal of streams, including defensive null checks and resetting state variables like InputStream.

Results

• The ObjectDisposedException no longer occurs under high-concurrency scenarios.
• The Pode server handles concurrent HTTPS requests more reliably without resource contention or premature disposal.

Impact

• Improved stability and reliability for HTTPS endpoints in Pode.
• Resolved a critical issue affecting high-concurrency scenarios, ensuring seamless handling of SSL/TLS operations.

Backward Compatibility

• These changes are fully backward-compatible with existing Pode functionality.

[Badgerati]

I don't see the need for having disposing and the deconstructor below?

If GC comes along, surely we want it to dispose of the Stream/Socket? And in the logic below, even if disposing=false we still set IsDisposed=true anyway 🤔

It's also better for IsDisposed=true to be set further up, before we actually dispose of objects, so we're not disposing of objects and then finally setting it to true - which does open the possibility for ObjectDisposedException as Receive could use them

[mdaneri]

You're absolutely right to question the need for both the destructor and the disposing = false logic. Since the class only deals with managed resources like streams and sockets, the destructor (~PodeRequest) isn't necessary.
Removing it simplifies things and avoids the disposing = false scenario altogether.
As for setting IsDisposed = true earlier—great point! Doing that upfront helps prevent potential ObjectDisposedException during concurrent use by marking the object as disposed before starting cleanup.
😊

[mdaneri]

regarding why to use Dispose(bool disposing)
https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose