Skip to content
This repository was archived by the owner on Apr 8, 2020. It is now read-only.

Allow proxy middleware to abort long running connections. #914

Closed

Conversation

mattvperry
Copy link
Contributor

The conditional proxy middleware that is used for connecting to the webpack dev server was leaving lingering connections when proxying long-running HTTP requests like those used for hot reloading.

This was happening because the CancellationTokenSource which lives on the HttpContext was never getting called after the initial SendAsync which completes after headers are sent.

The fix is simply to ensure that we respect the cancellation token when copying from the upstream response.

Note: I noticed this because when using .Net 4.6 plus System.Net.Http >= 4.3.1, you can only have 2 concurrent HttpClient requests active at a time because this change: dotnet/corefx#15036 uses the HttpClientHandler.MaxConnectionsPerServer with a default of 2. What was happening was that if I reloaded the page once or twice the hot module reloading wouldn't connect to webpack because it wouldn't let any more connections through because there were a couple lingering threads. It also might be worth changing HttpClientHandler.MaxConnectionsPerServer to something higher because even with this change the connections don't close immediately so several quick reloads can get you in the same position. Picking that default concurrency was out of scope for this PR though, I felt.

@dnfclas
Copy link

dnfclas commented May 4, 2017

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@mattvperry
Copy link
Contributor Author

I can also port this over to https://github.com/aspnet/Proxy if you guys think that's a good idea.

@SteveSandersonMS
Copy link
Member

Thanks @mattvperry! This is great. It's now merged into dev and will be released in an updated package shortly.

@mattvperry mattvperry deleted the mp/conditional_proxy_cleanup branch May 4, 2017 17:17
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.

3 participants