Fix threading issue when reading and writing from SslStream at the same time#37736
Fix threading issue when reading and writing from SslStream at the same time#37736krwq merged 1 commit intodotnet:masterfrom
Conversation
This is already prevented at the SslStream layer, ensuring that there's at most only one read and one write at a time. Did you actually see problems here, or is this speculation? |
|
|
|
/azp run corefx-outerloop-linux |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
There seem to be some issue with Azure DevOps - it reports build as red but nothing actually failed on the latest retry: https://dev.azure.com/dnceng/public/_build/results?buildId=191964 |
…me time (dotnet/corefx#37736) Commit migrated from dotnet/corefx@a28444b
Fixes #36426 - note, this is not scoped to Alpine - issue repros on any Linux with latest (1.1.1) OpenSSL installed.
There were also occasional process crashes I've observed disappearing after fixing this (didn't get a chance to capture stack trace as it happened fairly rarely and I initially thought it was due to bad setup/unrelated)
Details:
SSL_read, SSL_write, SSL_get_error are advertised as they are not designed to work in multithreaded scenarios when working on the same SSL connection: https://www.openssl.org/docs/faq.html#PROG1
for HTTP 1 scenarios this is rather unusual to read and write at the same time but for HTTP 2 this is common.
Note: multiple reads OR writes (but not reads and writes) at the same time can still cause some issues around BIO_read/BIO_write (BIO_* use separate queues for reads and writes) - I have not addressed them because multiple reads at the same time do not seem to be useful in any scenarios (user will essentially get random bytes since you can't reason about the order).
This is marked as draft because of the pending investigation:
cc: @wfurt @stephentoub
Before
After
End to end tests (aspnet/Benchmarks: PlaintextNonPipelined)
Numbers seem to have difference within the error margin which seems like it might not be worth to investigate further and just take a fix (we might want to check also more end to end scenarios)
cc: @sebastienros