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

Fix aspnet/AspNetCore-Internal#1869 #1215

Merged
merged 1 commit into from
Mar 4, 2019
Merged

Conversation

rynowak
Copy link
Member

@rynowak rynowak commented Mar 4, 2019

I think the issue here is that the HttpClient goes out of scope too
early and the cleanup pass is able to dispose it before we intend for it
to be disposed.

GC.Keepalive will preserve the liveness until the point we're ready to
null it out.

I think the issue here is that the HttpClient goes out of scope too
early and the cleanup pass is able to dispose it before we intend for it
to be disposed.

GC.Keepalive will preserve the liveness until the point we're ready to
null it out.
@rynowak rynowak requested a review from javiercn March 4, 2019 16:35
// the handler from being disposed if it was still rooted.
Assert.Empty(factory.ActiveEntryState);

// Act - 1 - Run a cleanup cycle, this will not dispose the handler, because the client is still live.
Copy link
Member Author

Choose a reason for hiding this comment

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

The error is coming from L493 and basically says that L490 was able to dispose the message handler. It's not intended that L490 dispose the message handler, it's supposed to survive this cycle - which implies that client1 been GCed already.

@rynowak rynowak merged commit e658402 into master Mar 4, 2019
@rynowak rynowak deleted the rynowak/http-flake-factory branch March 4, 2019 20:25
@ghost ghost locked as resolved and limited conversation to collaborators May 29, 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.

2 participants