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

PREVIEW: The new async-ified HttpWebRequest. #5200

Closed
wants to merge 11 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@baulig
Member

baulig commented Jul 12, 2017

[System]: Async-ify and mostly rewrite HttpWebRequest and friends.

HttpWebRequest and its helper classes (WebConnection, WebConnectionStream) have received a major overhaul.

  • The old Begin/End async based implementation has been rewritten to use async tasks. All public API entry points using the APM are now wrappers around their TPL counterparts.
  • WebConnectionStream has been split into WebRequestStream and WebResponseStream. All the reading and writing functionality that was previously spread across WebConnection, WebConnectionStream and HttpWebRequest now lives in these two new classes.
  • Each HttpWebRequest instance can transparently do more than one actual request due to redirection and / or authentication. These underlying requests are now called WebOperation and each instance of this new class handles exactly one network request.
  • The new WebOperation class also tracks state regarding sending / receiving / closing streams.
  • Most of the old functionality from WebConnection has been moved to other classes. WebConnection is now responsible for connecting to the remote server and keeping track of that connection.
  • All the connection reuse / idle time / queuing logic has been moved into a new class called ServicePointScheduler, replacing WebConnectionGroup and the relevant code in ServicePoint.
  • ServicePointScheduler is fully async and keeps track of all currently open connections, all currently running or queued requests and idle connections. It will automatically clean itself up and exit when it's no longer used.
  • Added error checking, and timeouts.

There were two main objectives for this patch: fixing https://bugzilla.xamarin.com/show_bug.cgi?id=55872 and various issues that random people reported regarding nuget restore printing random error messages - while also modernizing the code-base in a way where it's easier to read, understand and maintain.

This code is very old, most of it was written in 2004 and 2012, and it was using a lot of chained APM async methods which were very hard to understand, debug and fix. The underlying issue in the bug was that some "EndSomething" was called after the request had been canceled and the connection reused, but some of the code didn't check whether the underlying state had changed.

PREVIEW: The new async-ified HttpWebRequest.
[System]: Async-ify and mostly rewrite HttpWebRequest and friends.

HttpWebRequest and its helper classes (WebConnection, WebConnectionStream) have received a major overhaul.

* The old Begin/End async based implementation has been rewritten to use async tasks. All public API entry points
  using the APM are now wrappers around their TPL counterparts.
* WebConnectionStream has been split into WebRequestStream and WebResponseStream. All the reading and writing
  functionality that was previously spread across WebConnection, WebConnectionStream and HttpWebRequest now
  lives in these two new classes.
* Each HttpWebRequest instance can transparently do more than one actual request due to redirection and / or
  authentication. These underlying requests are now called WebOperation and each instance of this new class
  handles exactly one network request.
* The new WebOperation class also tracks state regarding sending / receiving / closing streams.
* Most of the old functionality from WebConnection has been moved to other classes. WebConnection is now
  responsible for connecting to the remote server and keeping track of that connection.
* All the connection reuse / idle time / queuing logic has been moved into a new class called
  ServicePointScheduler, replacing WebConnectionGroup and the relevant code in ServicePoint.
* ServicePointScheduler is fully async and keeps track of all currently open connections, all currently
  running or queued requests and idle connections. It will automatically clean itself up and exit when it's no
  longer used.
* Added error checking, and timeouts.

There were two main objectives for this patch: fixing https://bugzilla.xamarin.com/show_bug.cgi?id=55872 and
various issues that random people reported regarding nuget restore printing random error messages - while also
modernizing the code-base in a way where it's easier to read, understand and maintain.

This code is very old, most of it was written in 2004 and 2012, and it was using a lot of chained APM async
methods which were very hard to understand, debug and fix. The underlying issue in the bug was that some
"EndSomething" was called after the request had been canceled and the connection reused, but some of the code
didn't check whether the underlying state had changed.

@baulig baulig requested a review from marek-safar as a code owner Aug 1, 2017

baulig and others added some commits Aug 18, 2017

Use Request.ProtocolVersion, not ServicePoint.ProtocolVersion (which …
…isn't set until we go the first response).

(cherry picked from commit 7855db2)
[sdb] Fix support for async debugging in optimized mode, roslyn gener…
…ates valuetype IAsyncStateMethod implementations. Fixes #58728. (#5476)

(cherry picked from commit 1904650)

@baulig baulig requested review from akoeplinger, kumpera and vargaz as code owners Sep 1, 2017

baulig added some commits Sep 14, 2017

[Mono.Security]: Add internal 'MonoTlsProviderFactory.InternalVersion…
…' constant. (#5568)

Internal version number (not in any way related to the TLS Version).

Used by the web-tests to check whether the current Mono contains certain
features or bug fixes.

Negative version numbers are reserved for martin work branches.
(cherry picked from commit 002fcd5)
[System]: SslStream.Flush() now flushes the underlying stream. Bug #5…
…7528. (#5569)

* Mono.Security.Interface.IMonoSslStream: removed Flush().

* Mono.Net.Security.MobileAuthenticatedStream: Flush() now calls `InnerStream.Flush ()'.

* System.Net.Security.SslStream: Flush() now calls `InnerStream.Flush ()'.

* System.Net.Security.SslStream: fix `CanRead`, `CanWrite` and `CanTimeout` logic.
(cherry picked from commit 56c2802)

@baulig baulig requested a review from DavidKarlas as a code owner Sep 14, 2017

@baulig

This comment has been minimized.

Show comment
Hide comment
@baulig

baulig Nov 29, 2017

Member

New PR: #6125

Member

baulig commented Nov 29, 2017

New PR: #6125

@baulig baulig closed this Nov 29, 2017

@akoeplinger akoeplinger deleted the martin-new-webstack branch Jun 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment