Skip to content

Reenable SocketsHttpHandlerTest_Http3_MsQuic.SendMoreThanStreamLimitRequestsConcurrently_LastWaits test #67419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 3, 2022
Merged

Reenable SocketsHttpHandlerTest_Http3_MsQuic.SendMoreThanStreamLimitRequestsConcurrently_LastWaits test #67419

merged 2 commits into from
May 3, 2022
+26 −26

Conversation

@rzikm
Copy link
Member

@rzikm rzikm commented Apr 1, 2022

Just check if test passes on CI

@ghost ghost assigned rzikm Apr 1, 2022
@ghost ghost added the area-System.Net.Http label Apr 1, 2022
@ghost
Copy link

ghost commented Apr 1, 2022

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

Issue Details

Just check if test passes on CI

Author: rzikm
Assignees: rzikm
Labels:

area-System.Net.Http
Milestone: -

@rzikm
Copy link
Member Author

rzikm commented Apr 1, 2022

I managed to reproduce this locally, will try to catch a dump to see what is going on

@rzikm
Copy link
Member Author

rzikm commented Apr 8, 2022

dump analysis shows that the lastRequestContentStarted task completion source is never set and that server task is "stuck" after disposing of the first stream. Will revisit this after we have proper async dispose of the MsQuicStream

@ghost ghost added the no-recent-activity label Apr 22, 2022
@ghost
Copy link

ghost commented Apr 22, 2022

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

@rzikm rzikm force-pushed the 55901-HTTP/3-SocketsHttpHandlerTest_Http3_MsQuicSendMoreThanStreamLimitRequestsConcurrently_LastWaitsstreamLimit-10-fails branch from 7bbeb6a to b4d4e35 Compare April 27, 2022 11:56
@ghost ghost removed the no-recent-activity label Apr 27, 2022
@rzikm rzikm marked this pull request as ready for review April 27, 2022 16:21
@rzikm
Copy link
Member Author

rzikm commented Apr 27, 2022

Does not reproduce anymore after #67859

@rzikm rzikm requested a review from CarnaViire April 27, 2022 16:22
CarnaViire
CarnaViire approved these changes May 3, 2022
View reviewed changes
Copy link
Member

@CarnaViire CarnaViire left a comment

Choose a reason for hiding this comment

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

LGTM. Let's try it. Could you please take a closer look at this test in CI for some time to be sure it wouldn't start failing/hanging?

@rzikm @CarnaViire
Update src/libraries/System.Net.Http/tests/FunctionalTests/HttpClient…
badd60b 
…HandlerTest.Http3.cs

Co-authored-by: Natalia Kondratyeva <[email protected]>
@rzikm
Copy link
Member Author

rzikm commented May 3, 2022

I am still monitoring all test failures so if this starts happening again, I will hopefully notice it :)

@rzikm
Copy link
Member Author

rzikm commented May 3, 2022

CI Failures are unrelated (LibraryImportGenerator.Unit.Tests)

@rzikm rzikm merged commit ec28cb3 into dotnet:main May 3, 2022
@rzikm rzikm mentioned this pull request May 3, 2022
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jun 2, 2022
@karelz karelz added this to the 7.0.0 milestone Jul 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@CarnaViire CarnaViire CarnaViire approved these changes

Assignees

@rzikm rzikm

Labels

area-System.Net.Http

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@rzikm @CarnaViire @karelz