Lock when writing to an SslStream #418
Conversation
Network stream supports socket writes from multiple threads. SslStream does not hence we need to lock before writing when using Ssl. [#157088466]
| @@ -238,7 +237,7 @@ public void Close() | |||
| { | |||
| if (Model.IsOpen) | |||
| { | |||
| lock(m_basicCancelLock) | |||
| lock(Model) | |||
There was a problem hiding this comment.
I'm not sure this is a good idea. Model is passed in to Subscription, and it's a public property. That means that anything could potentially take a lock on the Model instance, and interfere with this code.
There was a problem hiding this comment.
👍 I'm not sure you should ever take a lock on a public reference. It's deadlock territory.
There was a problem hiding this comment.
Probably not a good idea as you say. We could lock on something inside the Model instead. I guess all rpcs inside a need to be locked over using a single lockobject per model as they cannot be pipelined.
As rpc methods cannot be pipelined we may as well lock around them. [#157088466]
kjnilsson
force-pushed
the
rabbtimq-dotnet-client-417
branch
from
546ea94 to
c428e03
Compare
|
This PR also includes locking around each RPC methods. As we cannot pipeline RPC calls we might as well lock around it. Combined with writing full frames this should increase model thread safety a fair bit. |
kjnilsson
changed the title
Network stream supports socket writes from multiple threads. SslStream
does not hence we need to lock before writing when using Ssl.
[#157088466]
May fix #417