Transport agnostic kestrel refactoring #1551

halter73 · 2017-03-24T18:15:16Z

What I plan to do as soon as I'm physically capable so this can be quickly merged to unblock people

Get unit tests compiling and passing again
Get performance project compiling and running again
Run perf tests

Possible issues/work items

Quickish fixes

Fix API check (exceptions.json)
Rewrite RequestHeaderLimitsTests "functional" tests to be unit tests or not access Frame
Fix namespaces/folders/class names (rename Microsoft.AspNetCore.Server.Kestrel.Libuv to Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv)
Create Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions project
Add doc comments for new public classes and interfaces
Don't use KestrelTrace class from Libuv project
Re-enable KestrelEventSource

Bigger work items

Move shutdown logic to Core and make it tranport agnostic
Move timeout logic to Core and make it tranport agnostic
Remove the IConnectionInformation.ConnectionControl (helped by above items)
Clean up IConnectionContext methods
Share KestrelThreads (LibuvThreads) between endpoints (will be easier when shutdown logic is moved)
Split transport-specific tests and general tests into distinct test projects
Write managed socket transport to truly test the abstraction

Notes:

The Https project no longer depends on the meta project which contains UseKestrel(). This is a breaking change.

#828

davidfowl · 2017-03-24T19:44:12Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/SocketOutputProducer.cs

-                    _log.ConnectionError(_connectionId, error);
-                }
+                _completed = true;
+                _pipe.Complete();


Don't call Complete in the lock. This can dispatch.

natemcmaster · 2017-03-24T21:17:31Z

KestrelHttpServer.sln

@@ -61,6 +61,10 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "TestResources", "TestResour
 		test\shared\TestResources\testCert.pfx = test\shared\TestResources\testCert.pfx
 	EndProjectSection
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server.Kestrel.Libuv", "src\Microsoft.AspNetCore.Server.Kestrel.Libuv\Microsoft.AspNetCore.Server.Kestrel.Libuv.csproj", "{A76B8C8C-0DC5-4DD3-9B1F-02E51A0915F4}"


As long as we're moving all of the code into different folders, would this be a good time to make project folder names shorter? Now that we're off PJ we can shorten folder names. Improves browsing code on GitHub and easier to use on commandline.

No 😄 https://github.com/thecodejunkie/github.expandinizr

natemcmaster · 2017-03-24T21:22:19Z

test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestHeaderLimitsTests.cs

-                }
-            }
-        }
+        //[Theory]


Theory(Skip = "reason")...unless this was intentional to avoid compiler errors.

This is to avoid compilation errors. FrameFactory no longer exists. This is what I'm referencing in the PR description where I mention:

Rewrite RequestHeaderLimitsTests "functional" tests to be unit tests or not access Frame

Just to be clear, you're talking about only rewriting the ones that access Frame, right? We should still have functional tests for things like timeouts.

Yes. I only disabled two tests that both accessed server.Frame.RequestHeaders.Count.

cesarblum · 2017-03-24T22:00:31Z

src/Microsoft.AspNetCore.Server.Kestrel.Libuv/Microsoft.AspNetCore.Server.Kestrel.Libuv.csproj

@@ -0,0 +1,23 @@
+<Project Sdk="Microsoft.NET.Sdk">


Since we've agreed to name transport packages as Microsoft.AspNetCore.Server.Kestrel.Transport.*, let's do this now do avoid further loss of history on renames.

cesarblum · 2017-03-24T22:01:19Z

src/Microsoft.AspNetCore.Server.Kestrel.Libuv/Microsoft.AspNetCore.Server.Kestrel.Libuv.csproj

+
+  <PropertyGroup>
+    <Description>Libuv transport for the ASP.NET Core Kestrel cross-platform web server.</Description>
+    <TargetFrameworks>net451;netstandard1.3</TargetFrameworks>


cesarblum · 2017-03-24T22:05:45Z

samples/SampleApp/SampleApp.csproj

@@ -12,6 +12,8 @@
  </ItemGroup>

  <ItemGroup>
+    <!-- REVIEW: Having the Https project depend on the non-Core project is breaking since it no longer transitively pulls in UseKestrel() :( -->


Remember to remove this comment when this is addressed.

I think that we'll have to leave this as an intentional breaking change. I just wanted to make people aware.

cesarblum · 2017-03-25T00:05:34Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/ConnectionLifetimeContext.cs

+        public Frame Frame { get; set; }
+        public SocketOutputProducer OutputProducer { get; set; }
+
+


nit: extra blank line

cesarblum · 2017-03-25T00:13:51Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/SocketOutputProducer.cs

 using Microsoft.Extensions.Internal;

 namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
 {
-    public class SocketOutput : ISocketOutput
+    public class SocketOutputProducer : ISocketOutput


Since we're no longer interacting directly with a socket, rename this to OutputProducer. Rename the interface too.

cesarblum · 2017-03-25T00:21:26Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServerOptions.cs

@@ -30,7 +30,7 @@ public class KestrelServerOptions

        /// <summary>
        /// Enables the Listen options callback to resolve and use services registered by the application during startup.
-        /// Typically initialized by <see cref="Hosting.WebHostBuilderKestrelExtensions.UseKestrel(Hosting.IWebHostBuilder, Action{KestrelServerOptions})"/>.
+        /// Typically initialized by UseKestrel()"/>.


Can we put this class in the non-core package?

This all becomes transport specific hmmm.

You wan't to put KestrelServerOptions in the meta package or Core couldn't read those options. The UseKestrel() extension method needs to be in the meta package so it can reference both Core and configure the default libuv transport.

@davidfowl Do you think we should have a WebHostBuilder extension method for using Kestrel without wiring the default libuv transport? What should it be called?

cesarblum · 2017-03-25T00:33:28Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/ConnectionLifetime.cs

+
+namespace Microsoft.AspNetCore.Server.Kestrel.Internal
+{
+    public class ConnectionLifetime : IConnectionContext


I think this class should be named Connection, or better HttpConnection to avoid ambiguity with transport connection classes.

The reason I think this one shouldn't have Lifetime in its name is that it does more than lifetime stuff e.g. exposes I/O stuff. OTOH the other class exposes just End, which is clearly related to the connection's lifetime.

FrameConnection?

davidfowl · 2017-03-25T03:28:39Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServer.cs

 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;

 namespace Microsoft.AspNetCore.Server.Kestrel
 {
    public class KestrelServer : IServer
    {
-        private Stack<IDisposable> _disposables;
-        private readonly IApplicationLifetime _applicationLifetime;
+        //private Stack<IDisposable> _disposables;


davidfowl · 2017-03-25T03:42:23Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/ConnectionHandler.cs

+        {
+            _serviceContext = serviceContext;
+            _application = application;
+            _pipeFactory = new PipeFactory();


This needs to be passed into OnConnection. The transport may want a pool per thread (in the case of libuv), or may want to affinitize the memory to a specific connection.

davidfowl · 2017-03-25T03:53:09Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServer.cs

-                _disposables.Push(engine.CreateServer(ipv4ListenOptions));
+                var transport = _transportFactory.Create(ipv4ListenOptions, connectionHandler);
+                _transports.Add(transport);
+                transport.BindAsync().Wait();


Do we really need to .Wait?

For backwards compatibility we do. BindAsync() can throw AddressInUse exceptions for example.

Can you use await instead?

davidfowl · 2017-03-25T03:53:55Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Transport/IConnectionHandler.cs

+{
+    public interface IConnectionHandler
+    {
+        IConnectionContext OnConnection(IConnectionInformation connectionInfo, IScheduler inputWriterScheduler, IScheduler outputReaderScheduler);


Add the PipeFactory here

davidfowl · 2017-03-25T04:01:59Z

src/Microsoft.AspNetCore.Server.Kestrel.Libuv/Internal/Http/Connection.cs

+            catch (Exception e)
+            {
+                Log.LogError(0, e, "Connection.StartFrame");
+                throw;


This is interesting, do we catch this elsewhere?

I copied this from old code, but Listener.OnConnection() should catch this if it isn't caught here.

davidfowl · 2017-03-25T04:04:57Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Transport/IConnectionContext.cs

+        // TODO: Remove these (Use Pipes instead?)
+        Task StopAsync();
+        void Abort(Exception ex);
+        void SetBadRequestState(RequestRejectionReason reason);


Change this to Timeout() and keep the RequestRejectionReason in Frame.

I told you about this Thursday, and you told me to keep it lol. Obvs should change though.

davidfowl · 2017-03-25T04:06:07Z

src/Microsoft.AspNetCore.Server.Kestrel.Libuv/Internal/Http/Connection.cs

+                ConnectionId = _connectionContext.ConnectionId;
+
+                Log.ConnectionStart(ConnectionId);
+                KestrelEventSource.Log.ConnectionStart(this);


We should move this to ConnectionHandler.OnConnection

Agreed. I had

Re-enable KestrelEventSource

in the original description, because i had it commented out for a while until I could fix it. I might file an issue to do this after merged if not trivial.

davidfowl · 2017-03-25T04:21:02Z

src/Microsoft.AspNetCore.Server.Kestrel.Libuv/Internal/LibuvTransportContext.cs

+
+        public IApplicationLifetime AppLifetime { get; set; }
+
+        public IKestrelTrace Log { get; set; }


// TODO: Remove

We'll keep something like this, but it won't be (I)KestrelTrace.

davidfowl · 2017-03-25T04:24:49Z

src/Microsoft.AspNetCore.Server.Kestrel.Libuv/Internal/Infrastructure/KestrelThread.cs

@@ -92,8 +89,6 @@ internal KestrelThread(KestrelEngine engine, int maxLoops)

        public MemoryPool Memory { get; }

-        public PipeFactory PipelineFactory { get; }


This should stay here

davidfowl · 2017-03-25T04:25:31Z

src/Microsoft.AspNetCore.Server.Kestrel.Libuv/Internal/Http/SocketOutputConsumer.cs

+        private readonly WriteReqPool _writeReqPool;
+        private readonly IPipeReader _pipe;
+
+        // https://github.com/dotnet/corefxlab/issues/1334 


This belongs in SocketOutputProducer

davidfowl · 2017-03-25T08:19:58Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/ConnectionLifetime.cs

+            {
+                Log.LogError(0, ex, $"Uncaught exception from the {nameof(IConnectionAdapter.OnConnectionAsync)} method of an {nameof(IConnectionAdapter)}.");
+                _frameStartedTcs.SetResult(null);
+                CloseRawPipes();


When this happens does the SocketOutputConsumer end as well?

I doesn't. This isn't the consuming class. The closing of the pipes was different for adapters and raw pipes prior to this PR. I just kept existing behavior that we used to have in Connection.OnSocketClosed(). We can clean up in another PR.

davidfowl · 2017-03-25T11:09:47Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/Frame.cs

-                    Log.LogError(0, ex, "Abort");
-                }
+                // Potentially calling user code. ThreadPool will catch and log any errors.
+                ServiceContext.ThreadPool.Run(() => RequestAbortedSource.Cancel());


Thread pool isn't pluggable anymore right?

Dispatching user code or not is still selectable using InternalKestrelServerOptions

davidfowl · 2017-03-25T11:10:26Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/FrameOfT.cs

@@ -190,15 +193,18 @@ public Frame(IHttpApplication<TContext> application, ConnectionContext context)
                // SetBadRequestState logs the error.
                SetBadRequestState(ex);
            }
-            catch (IOException ex) when (ex.InnerException is UvException)
+            catch (ConnectionResetException ex)


I like this alot 👍

davidfowl · 2017-03-25T11:12:38Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/SocketOutputProducer.cs

                if (_completed)
                {
+                    // TODO: Get actual notification when the consumer stopped from Pipes,
+                    // so we know if the socket is fully closed and why (for logging exceptions);
+                    _log.ConnectionDisconnectedWrite(_connectionId, buffer.Count, ex: null);


File a bug for this

davidfowl · 2017-03-25T11:28:40Z

samples/SampleApp/Startup.cs

@@ -61,9 +61,11 @@ public static void Main(string[] args)

                    // The following section should be used to demo sockets
                    //options.ListenUnixSocket("/tmp/kestrel-test.sock");
-
+                })
+                .UseLibuv(options =>


It's weird to have this on the same level as kestrel. This needs to move to options.

That just means that hosting services don't get injected into the ITransportFactory for free. For example, LibuvTransportFactory depends on IOptions<LibuvTransportOptions>, IApplicationLifetimeandILoggerFactory`.

Do you want to use ActivatorUtilities to activate a registered ITransportFactory type on KestrelServerOptions? That seems really gross.

benaadams · 2017-03-25T22:02:17Z

src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServer.cs

-                _disposables.Push(engine.CreateServer(ipv6ListenOptions));
+                var transport = _transportFactory.Create(ipv6ListenOptions, connectionHandler);
+                _transports.Add(transport);
+                transport.BindAsync().Wait();


#1551 (comment)

cesarblum · 2017-03-25T04:24:30Z

src/Microsoft.AspNetCore.Server.Kestrel.Libuv/Internal/Http/Connection.cs

+            }
+            catch (Exception e)
+            {
+                Log.LogError(0, e, "Connection.StartFrame");


nameof, especially now that things are being moved around and renamed.

The names are often wrong and could muck up the PR.

In the current code I can find 26 LogError calls that could use nameof, but don't. I would change them all together

davidfowl · 2017-03-29T09:42:41Z

API check LOL 😄

- All existing code stays in the Core package for now

- Rename Microsoft.AspNetCore.Server.Kestrel.Libuv to Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv

cesarblum · 2017-03-29T23:12:56Z

🎉

natemcmaster · 2017-03-29T23:21:18Z

👏

halter73 · 2017-03-29T23:32:38Z

I ran all the BenchmarkDotNet benchmarks, and the numbers for those stayed very stable before and after the change. This makes sense considering those don't test the integration between the transport and parsing code.

I ran the end-to-end techempower benchmark on 2 F4 linux VMs, and the results were comparable:

Before

shalter@f4-linux:~$ wrk -H 'Host: localhost' -H 'Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7' -H 'Connection: keep-alive' --latency -d 15 -c 256 --timeout 8 -t 32 http ://10.1.2.6:5000/plaintext -s ~/pipeline.lua -- 16
Running 15s test @ http://10.1.2.6:5000/plaintext
  32 threads and 256 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     6.14ms   12.15ms 229.67ms   98.34%
    Req/Sec    16.58k     2.61k   38.33k    85.11%
  Latency Distribution
     50%    4.42ms
     75%    7.07ms
     90%   10.37ms
     99%   24.65ms
  7662062 requests in 14.91s, 0.94GB read
  Socket errors: connect 0, read 0, write 0, timeout 669
Requests/sec: 514007.22
Transfer/sec:     64.71MB
shalter@f4-linux:~$ wrk -H 'Host: localhost' -H 'Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7' -H 'Connection: keep-alive' --latency -d 15 -c 256 --timeout 8 -t 32 http ://10.1.2.6:5000/plaintext -s ~/pipeline.lua -- 16
Running 15s test @ http://10.1.2.6:5000/plaintext
  32 threads and 256 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     5.37ms    4.04ms  54.98ms   74.30%
    Req/Sec    16.58k     2.58k   33.12k    81.80%
  Latency Distribution
     50%    4.42ms
     75%    7.35ms
     90%   10.70ms
     99%   18.41ms
  7859493 requests in 14.91s, 0.97GB read
  Socket errors: connect 0, read 0, write 0, timeout 761
Requests/sec: 527223.63
Transfer/sec:     66.37MB
shalter@f4-linux:~$ wrk -H 'Host: localhost' -H 'Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7' -H 'Connection: keep-alive' --latency -d 15 -c 256 --timeout 8 -t 32 http ://10.1.2.6:5000/plaintext -s ~/pipeline.lua -- 16
Running 15s test @ http://10.1.2.6:5000/plaintext
  32 threads and 256 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     5.38ms    3.90ms  58.41ms   71.52%
    Req/Sec    16.42k     2.19k   46.99k    83.27%
  Latency Distribution
     50%    4.46ms
     75%    7.48ms
     90%   10.75ms
     99%   16.91ms
  7795328 requests in 14.91s, 0.96GB read
  Socket errors: connect 0, read 0, write 0, timeout 765
Requests/sec: 522778.47
Transfer/sec:     65.81MB

After

shalter@f4-linux:~$ wrk -H 'Host: localhost' -H 'Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7' -H 'Connection: keep-alive' --latency -d 15 -c 256 --timeout 8 -t 32 http ://10.1.2.6:5000/plaintext -s ~/pipeline.lua -- 16
Running 15s test @ http://10.1.2.6:5000/plaintext
  32 threads and 256 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     6.25ms   14.37ms 258.17ms   98.51%
    Req/Sec    16.79k    22.40k    1.10M    99.96%
  Latency Distribution
     50%    4.32ms
     75%    6.77ms
     90%    9.99ms
     99%   31.72ms
  7759079 requests in 14.89s, 0.95GB read
  Socket errors: connect 0, read 0, write 0, timeout 767
Requests/sec: 521248.89
Transfer/sec:     65.62MB
shalter@f4-linux:~$ wrk -H 'Host: localhost' -H 'Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7' -H 'Connection: keep-alive' --latency -d 15 -c 256 --timeout 8 -t 32 http ://10.1.2.6:5000/plaintext -s ~/pipeline.lua -- 16
Running 15s test @ http://10.1.2.6:5000/plaintext
  32 threads and 256 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     4.98ms    3.51ms  69.46ms   73.95%
    Req/Sec    16.70k     2.22k   38.88k    81.03%
  Latency Distribution
     50%    4.34ms
     75%    6.70ms
     90%    9.29ms
     99%   16.65ms
  7924464 requests in 14.91s, 0.97GB read
  Socket errors: connect 0, read 0, write 0, timeout 766
Requests/sec: 531397.56
Transfer/sec:     66.89MB
shalter@f4-linux:~$ wrk -H 'Host: localhost' -H 'Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7' -H 'Connection: keep-alive' --latency -d 15 -c 256 --timeout 8 -t 32 http ://10.1.2.6:5000/plaintext -s ~/pipeline.lua -- 16
Running 15s test @ http://10.1.2.6:5000/plaintext
  32 threads and 256 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     5.34ms    3.89ms  69.39ms   72.32%
    Req/Sec    16.60k     2.50k   43.53k    79.97%
  Latency Distribution
     50%    4.44ms
     75%    7.34ms
     90%   10.64ms
     99%   17.58ms
  7875381 requests in 14.91s, 0.97GB read
  Socket errors: connect 0, read 0, write 0, timeout 767
Requests/sec: 528140.19
Transfer/sec:     66.48MB

aspnet/KestrelHttpServer#1551

tmds · 2017-03-30T07:01:23Z

👍 🎉

dnfclas added the cla-already-signed label Mar 24, 2017

davidfowl reviewed Mar 24, 2017

View reviewed changes

natemcmaster reviewed Mar 24, 2017

View reviewed changes

natemcmaster mentioned this pull request Mar 24, 2017

Short folder names #1555

Merged

cesarblum reviewed Mar 25, 2017

View reviewed changes

davidfowl reviewed Mar 25, 2017

View reviewed changes

benaadams reviewed Mar 25, 2017

View reviewed changes

davidfowl mentioned this pull request Mar 26, 2017

Refactored some functional tests. #1561

Closed

cesarblum reviewed Mar 27, 2017

View reviewed changes

natemcmaster mentioned this pull request Mar 27, 2017

Add tests for WritableBuffer extensions and rename WriteAscii => WriteAsciiNoValidation #1553

Merged

cesarblum mentioned this pull request Mar 27, 2017

Use nameof where possible #1563

Closed

halter73 force-pushed the halter73/828 branch from 876837c to fe93ca3 Compare March 29, 2017 09:29

halter73 added 2 commits March 29, 2017 12:38

Create Libuv and Core projects

07e7296

- All existing code stays in the Core package for now

Add transport interfaces

b953daa

halter73 added 14 commits March 29, 2017 12:38

WIP 3

8238bac

WIP 4

4c5b085

WIP 5

b4c60a9

WIP 6

2a8ecd6

WIP 7

0567559

WIP 8

ef3b033

WIP 9

a90b1e7

WIP 10

af243e0

WIP 11

0a86f0c

WIP 12

236fea5

WIP 13

9c2f5d9

WIP 14

ba1b4c1

WIP 15

45fd2a5

- Rename Microsoft.AspNetCore.Server.Kestrel.Libuv to Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv

WIP 16

86aa930

halter73 force-pushed the halter73/828 branch from fe93ca3 to 86aa930 Compare March 29, 2017 19:39

halter73 added 2 commits March 29, 2017 13:53

WIP 16

711faa4

WIP 17

da3c028

natemcmaster mentioned this pull request Mar 29, 2017

Add Connection as a primary request header #1569

Merged

halter73 changed the title ~~[WIP] Transport agnostic kestrel refactoring~~ Transport agnostic kestrel refactoring Mar 29, 2017

davidfowl approved these changes Mar 29, 2017

View reviewed changes

cesarblum mentioned this pull request Mar 29, 2017

Update benchmarks code after Kestrel's transport-agnostic refactoring aspnet/Benchmarks#240

Closed

halter73 merged commit 7f78558 into dev Mar 29, 2017

halter73 deleted the halter73/828 branch March 29, 2017 23:35

halter73 added a commit to aspnet/Coherence that referenced this pull request Mar 30, 2017

Update packages.csv to include new Kestrel packages

7186a32

aspnet/KestrelHttpServer#1551

halter73 mentioned this pull request Mar 30, 2017

Update packages.csv to include new Kestrel packages aspnet/Coherence#178

Merged

halter73 added a commit to aspnet/Coherence that referenced this pull request Mar 30, 2017

Update packages.csv to include new Kestrel packages

68fb430

aspnet/KestrelHttpServer#1551

		public Frame Frame { get; set; }
		public SocketOutputProducer OutputProducer { get; set; }


		public IApplicationLifetime AppLifetime { get; set; }

		public IKestrelTrace Log { get; set; }

		@@ -92,8 +89,6 @@ internal KestrelThread(KestrelEngine engine, int maxLoops)

		public MemoryPool Memory { get; }

		public PipeFactory PipelineFactory { get; }

Transport agnostic kestrel refactoring #1551

Transport agnostic kestrel refactoring #1551

Conversation

halter73 commented Mar 24, 2017

What I plan to do as soon as I'm physically capable so this can be quickly merged to unblock people

Possible issues/work items

Quickish fixes

Bigger work items

Notes:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidfowl Mar 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidfowl Mar 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidfowl Mar 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

halter73 Mar 27, 2017 • edited Loading

Choose a reason for hiding this comment

davidfowl Mar 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidfowl commented Mar 29, 2017

cesarblum commented Mar 29, 2017

natemcmaster commented Mar 29, 2017

halter73 commented Mar 29, 2017

Before

After

tmds commented Mar 30, 2017

davidfowl Mar 25, 2017 •

edited

Loading

davidfowl Mar 25, 2017 •

edited

Loading

davidfowl Mar 25, 2017 •

edited

Loading

halter73 Mar 27, 2017 •

edited

Loading

davidfowl Mar 25, 2017 •

edited

Loading