Skip to content
This repository was archived by the owner on Jul 9, 2023. It is now read-only.

CustomBufferedStream were not disposed #590

Merged
merged 7 commits into from
May 11, 2019

Conversation

StephaneGraziano
Copy link
Contributor

Doneness:

  • Build is okay - I made sure that this change is building successfully.
  • No Bugs - I made sure that this change is working properly as expected. It doesn't have any bugs that you are aware of.
  • Branching - If this is not a hotfix, I am making this request against the master branch

The previous code :

// 1st clientStream
var clientStream = new CustomBufferedStream(clientConnection.GetStream(), BufferPool, BufferSize);
sslStream = new SslStream(clientStream, true); //true => do not dispose 1st ClientStream
// 2nd clientStream
clientStream = new CustomBufferedStream(sslStream, BufferPool, BufferSize);

...

sslStream?.Dispose(); // dispose sslStream but not 1st ClientStream
clientStream.Dispose(); // dispose 2nd ClientStream + sslStream (already disposed)
// here 1st clientStream is not disposed

The workaround is to handle the dispose of 1st clientStream inside SslStream.

sslStream = new SslStream(clientStream, false);

… for an unknown reason. In any case, this code is easier to read.
…ufferpool (to get the best of the two worlds)
---------------------------
//1st clientStream
var clientStream = new CustomBufferedStream(clientConnection.GetStream(), BufferPool, BufferSize);
sslStream = new SslStream(clientStream, false); //false => do not dispose 1st ClientStream
//2nd clientStream
clientStream = new CustomBufferedStream(sslStream, BufferPool, BufferSize);

...

sslStream?.Dispose(); // dispose sslStream but not 1st ClientStream
clientStream.Dispose(); // dispose 2nd ClientStream, sslStream (already disposed)
//here 1st clientStream is not disposed
---------------------------
@justcoding121 justcoding121 merged commit d663890 into justcoding121:master May 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants