Skip to content

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented May 7, 2025

Backport of #113124 to release/8.0-staging

Fixes #109689

/cc @rzikm

Customer Impact

Reported by 2 customers. They experience process crashes when using specific pattern with short HTTP request timeout.

The crash happens when they dispose SslStream concurrently with an ongoing handshake operation on Linux. The concurrent dispose may occur when using HttpClient with very short timeout settings.

Regression

No -- the scenario is not too common and the timing in TLS 1.3 might be different from previous TLS versions, affecting chances of the problem happening.

Testing

Successfully reproduced on main with modified Runtime (artificial sleeps in code to increase likelihood of the race condition).
We lost customer repro environment after they updated OS and OpenSSL versions - we cannot validate on their side.

Risk

Low. The change is well contained and uses existing patterns.

@rzikm rzikm self-assigned this May 9, 2025
@karelz karelz added this to the 8.0.x milestone May 28, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

@karelz karelz added the Servicing-consider Issue for next servicing release review label May 28, 2025
@karelz
Copy link
Member

karelz commented May 28, 2025

Reliability problem (crash) reported by 2 customers (one of them on 8.0). Worth a fix.

@rzikm rzikm requested review from wfurt and bartonjs June 4, 2025 13:25
Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. should we port the test as well for consistency?

Copy link
Member

@bartonjs bartonjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Tomas, seems like the test should have been included.

rzikm and others added 3 commits June 5, 2025 16:19
* Defer RemoteCertificate assignment after X509 Chain build

* Add comment
* Fix SslStreamDisposeTest.Dispose_ParallelWithHandshake_ThrowsODE test failures

* Fix build
* [Test Failure] SslStreamDisposeTest.Dispose_ParallelWithHandshake_ThrowsODE on Unix
Fixes #113833

* fixup! [Test Failure] SslStreamDisposeTest.Dispose_ParallelWithHandshake_ThrowsODE on Unix Fixes #113833

* fixup! fixup! [Test Failure] SslStreamDisposeTest.Dispose_ParallelWithHandshake_ThrowsODE on Unix Fixes #113833

* Update src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamDisposeTest.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix build

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@rzikm
Copy link
Member

rzikm commented Jun 5, 2025

The test in the original PR was flaky and uncovered other issues, so I had to include more PR (mostli test fixes). It should be good now.

@jozkee
Copy link
Member

jozkee commented Jun 9, 2025

@rzikm reminder: servicing code complete for 8.0.18 is today @ 3PM PST. This is still missing tactics approval.

@jozkee jozkee added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jun 10, 2025
@jozkee
Copy link
Member

jozkee commented Jun 10, 2025

Approved via email

@jozkee jozkee merged commit cbb1cd1 into release/8.0-staging Jun 10, 2025
114 of 117 checks passed
@akoeplinger akoeplinger deleted the backport/pr-113124-to-release/8.0-staging branch June 11, 2025 21:42
@github-actions github-actions bot locked and limited conversation to collaborators Jul 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants