-
Notifications
You must be signed in to change notification settings - Fork 191
Allow DefaultHttpContext to be pooled [Design] #404
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,13 +13,18 @@ | |
|
||
namespace Microsoft.AspNet.Http.Authentication.Internal | ||
{ | ||
public class DefaultAuthenticationManager : AuthenticationManager | ||
public class DefaultAuthenticationManager : AuthenticationManager, IDisposable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of using IDisposable to release memory for the pooled object, feels like mismatch... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps IReusable? With .Clear() method? |
||
{ | ||
private readonly IFeatureCollection _features; | ||
private IFeatureCollection _features; | ||
private FeatureReference<IHttpAuthenticationFeature> _authentication = FeatureReference<IHttpAuthenticationFeature>.Default; | ||
private FeatureReference<IHttpResponseFeature> _response = FeatureReference<IHttpResponseFeature>.Default; | ||
|
||
public DefaultAuthenticationManager(IFeatureCollection features) | ||
{ | ||
Initalize(features); | ||
} | ||
|
||
public void Initalize(IFeatureCollection features) | ||
{ | ||
_features = features; | ||
} | ||
|
@@ -109,5 +114,13 @@ public override async Task SignOutAsync([NotNull] string authenticationScheme, A | |
throw new InvalidOperationException($"The following authentication scheme was not accepted: {authenticationScheme}"); | ||
} | ||
} | ||
|
||
public void Dispose() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reset |
||
{ | ||
_features = null; | ||
|
||
_authentication.Reset(); | ||
_response.Reset(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
using System; | ||
using System.Net; | ||
using System.Security.Cryptography.X509Certificates; | ||
using System.Threading; | ||
|
@@ -10,14 +11,19 @@ | |
|
||
namespace Microsoft.AspNet.Http.Internal | ||
{ | ||
public class DefaultConnectionInfo : ConnectionInfo | ||
public class DefaultConnectionInfo : ConnectionInfo, IDisposable | ||
{ | ||
private readonly IFeatureCollection _features; | ||
private IFeatureCollection _features; | ||
|
||
private FeatureReference<IHttpConnectionFeature> _connection = FeatureReference<IHttpConnectionFeature>.Default; | ||
private FeatureReference<ITlsConnectionFeature> _tlsConnection = FeatureReference<ITlsConnectionFeature>.Default; | ||
|
||
public DefaultConnectionInfo(IFeatureCollection features) | ||
{ | ||
Initalize(features); | ||
} | ||
|
||
internal void Initalize(IFeatureCollection features) | ||
{ | ||
_features = features; | ||
} | ||
|
@@ -72,5 +78,13 @@ public override X509Certificate2 ClientCertificate | |
{ | ||
return TlsConnectionFeature.GetClientCertificateAsync(cancellationToken); | ||
} | ||
|
||
public void Dispose() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reset |
||
{ | ||
_features = null; | ||
|
||
_connection.Reset(); | ||
_tlsConnection.Reset(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,10 +16,10 @@ namespace Microsoft.AspNet.Http.Internal | |
{ | ||
public class DefaultHttpContext : HttpContext | ||
{ | ||
private readonly HttpRequest _request; | ||
private readonly HttpResponse _response; | ||
private readonly ConnectionInfo _connection; | ||
private readonly AuthenticationManager _authenticationManager; | ||
private readonly DefaultHttpRequest _request; | ||
private readonly DefaultHttpResponse _response; | ||
private readonly DefaultConnectionInfo _connection; | ||
private readonly DefaultAuthenticationManager _authenticationManager; | ||
|
||
private FeatureReference<IItemsFeature> _items; | ||
private FeatureReference<IServiceProvidersFeature> _serviceProviders; | ||
|
@@ -28,6 +28,7 @@ public class DefaultHttpContext : HttpContext | |
private FeatureReference<ISessionFeature> _session; | ||
private WebSocketManager _websockets; | ||
private IFeatureCollection _features; | ||
private Action<DefaultHttpContext> _onDisposing; | ||
|
||
public DefaultHttpContext() | ||
: this(new FeatureCollection()) | ||
|
@@ -36,6 +37,12 @@ public DefaultHttpContext() | |
_features.Set<IHttpResponseFeature>(new HttpResponseFeature()); | ||
} | ||
|
||
public DefaultHttpContext(IFeatureCollection features, Action<DefaultHttpContext> onDisposing) | ||
: this(features) | ||
{ | ||
_onDisposing = onDisposing; | ||
} | ||
|
||
public DefaultHttpContext(IFeatureCollection features) | ||
{ | ||
_features = features; | ||
|
@@ -51,6 +58,16 @@ public DefaultHttpContext(IFeatureCollection features) | |
_session = FeatureReference<ISessionFeature>.Default; | ||
} | ||
|
||
public void Reinitalize(IFeatureCollection features) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Initialize |
||
{ | ||
_features = features; | ||
|
||
_request.Initalize(this, features); | ||
_response.Initalize(this, features); | ||
_connection.Initalize(features); | ||
_authenticationManager.Initalize(features); | ||
} | ||
|
||
IItemsFeature ItemsFeature | ||
{ | ||
get { return _items.Fetch(_features) ?? _items.Update(_features, new ItemsFeature()); } | ||
|
@@ -168,8 +185,26 @@ public override void Abort() | |
|
||
public override void Dispose() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reset There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only one that doesn't fit as is already Disposable? |
||
{ | ||
// REVIEW: is this necessary? is the environment "owned" by the context? | ||
_features.Dispose(); | ||
_features = null; | ||
|
||
_request.Dispose(); | ||
_response.Dispose(); | ||
_connection.Dispose(); | ||
_authenticationManager.Dispose(); | ||
|
||
_items.Reset(); | ||
_serviceProviders.Reset(); | ||
_authentication.Reset(); | ||
_lifetime.Reset(); | ||
_session.Reset(); | ||
|
||
_websockets = null; | ||
|
||
if(_onDisposing != null) | ||
{ | ||
_onDisposing(this); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,14 +10,19 @@ | |
|
||
namespace Microsoft.AspNet.Http.Internal | ||
{ | ||
public class DefaultHttpResponse : HttpResponse | ||
public class DefaultHttpResponse : HttpResponse, IDisposable | ||
{ | ||
private readonly DefaultHttpContext _context; | ||
private readonly IFeatureCollection _features; | ||
private DefaultHttpContext _context; | ||
private IFeatureCollection _features; | ||
private FeatureReference<IHttpResponseFeature> _response = FeatureReference<IHttpResponseFeature>.Default; | ||
private FeatureReference<IResponseCookiesFeature> _cookies = FeatureReference<IResponseCookiesFeature>.Default; | ||
|
||
public DefaultHttpResponse(DefaultHttpContext context, IFeatureCollection features) | ||
{ | ||
Initalize(context, features); | ||
} | ||
|
||
internal void Initalize(DefaultHttpContext context, IFeatureCollection features) | ||
{ | ||
_context = context; | ||
_features = features; | ||
|
@@ -116,5 +121,14 @@ public override void Redirect(string location, bool permanent) | |
|
||
Headers[HeaderNames.Location] = location; | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reset |
||
_context = null; | ||
_features = null; | ||
|
||
_response.Reset(); | ||
_cookies.Reset(); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really need to hold onto FeatureReference structs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to check; they are heap allocated - so would be another chunk of memory but they are only a pointer and an int...