Skip to content
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.

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

Improve NonDisposedSocket_SafeHandlesCollected test #50104

Conversation

antonfirsov
Copy link
Member

@antonfirsov antonfirsov commented Mar 23, 2021

As reported in #50068, the test DisposedSocket.NonDisposedSocket_SafeHandlesCollected seems to intermittently hang the entire pipeline, although I believe it's a very rare case.

The problem is that currently there are no statistics on frequency and/or other starting points to diagnose it. This PR attempts to improve the test:

  • Failure should result in a timeout not a hang, so we can get better logs and Kusto stats
  • There may be two reasons: Hang in CreateHandlesAsync or hang in the finalizer. We need to be able to distinguish between those reasons.

@geoffkizer @wfurt thoughts?

@ghost
Copy link

ghost commented Mar 23, 2021

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

Issue Details

As reported in #50068, the test DisposedSocket.NonDisposedSocket_SafeHandlesCollected seems to intermittently hang the entire pipeline, although I believe it's a very rare case.

The problem is that currently any statistics or other starting points to diagnose it. This PR attempts to improve the test:

  • Failure should result in a timeout not a hang, so we can get logs and Kusto stats
  • There may be two reasons: Hang in CreateHandlesAsync or in the finalizer. We need to be able to distinguish between those reasons.

@geoffkizer @wfurt thoughts?

Author: antonfirsov
Assignees: -
Labels:

area-System.Net.Sockets

Milestone: -

@geoffkizer
Copy link
Contributor

I wonder if there should be some sort of debug-only logic in the Socket finalizer to try to detect that we are hanging. Like assert after a certain # of iterations?

Regardless, this change seems worthwhile to help track down what's happening here.

@antonfirsov
Copy link
Member Author

antonfirsov commented Mar 23, 2021

I wonder if there should be some sort of debug-only logic in the Socket finalizer to try to detect that we are hanging.

That makes sense but it should be a separate effort. Here I'm just trying to quickly bring #50068 to a somewhat better state, before someone just disables the test with a big hammer.

@antonfirsov antonfirsov merged commit 60037aa into dotnet:main Mar 24, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 23, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants