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

Simplify task handling in MetricCollector to avoid racy edge case #4121

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

geeknoid
Copy link
Member

@geeknoid geeknoid commented Jun 27, 2023

null

Microsoft Reviewers: Open in CodeFlow

@geeknoid geeknoid requested a review from tarekgh June 27, 2023 00:17
@ghost ghost assigned geeknoid Jun 27, 2023

_waiters.Clear();
// wake up anybody still waiting and inform them of the bad news: their horse is dead...
Copy link
Member

Choose a reason for hiding this comment

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

could you please add a comment in all places we complete the waiter.TaskSource telling need to be called outside the lock.

@RussKie
Copy link
Member

RussKie commented Jun 27, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@geeknoid geeknoid merged commit 5a8029d into main Jun 27, 2023
6 checks passed
@geeknoid geeknoid deleted the geeknoid/mc branch June 27, 2023 14:05
@ghost ghost added this to the 8.0 Preview7 milestone Jun 27, 2023
@geeknoid
Copy link
Member Author

@tarekgh I added the comments about usage of the task needing to be outside of the critical section.

Unfortunately, I needed to leave the Thread.Sleep in the code since the test case is still failing. There is still something asynchronous happening which we don't understand. Can I impose on you again to figure out what's happening please?

@tarekgh
Copy link
Member

tarekgh commented Jun 27, 2023

I'll try with the latest changes hopefully I can reproduce it. Looks it is failing on .NET 8.0 only now.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants