Cleanup - DisposableObject & disposing UmbracoContext

This commit is contained in:
Stephan
2016-06-09 19:06:49 +02:00
parent 5a839c3654
commit 9fe9f9d167
10 changed files with 44 additions and 42 deletions

View File

@@ -1,5 +1,4 @@
using System;
using System.Threading;
namespace Umbraco.Core
{
@@ -8,24 +7,22 @@ namespace Umbraco.Core
/// </summary>
/// <remarks>
/// Can also be used as a pattern for when inheriting is not possible.
///
///
/// See also: https://msdn.microsoft.com/en-us/library/b1yfkh5e%28v=vs.110%29.aspx
/// See also: https://lostechies.com/chrispatterson/2012/11/29/idisposable-done-right/
///
///
/// Note: if an object's ctor throws, it will never be disposed, and so if that ctor
/// has allocated disposable objects, it should take care of disposing them.
/// </remarks>
public abstract class DisposableObject : IDisposable
{
private bool _disposed;
private readonly object _locko = new object();
// gets a value indicating whether this instance is disposed.
// for internal tests only (not thread safe)
//TODO make this internal + rename "Disposed" when we can break compatibility
public bool IsDisposed { get { return _disposed; } }
public bool Disposed { get; private set; }
// implements IDisposable
// implements IDisposable
public void Dispose()
{
Dispose(true);
@@ -38,13 +35,12 @@ namespace Umbraco.Core
Dispose(false);
}
//TODO make this private, non-virtual when we can break compatibility
protected virtual void Dispose(bool disposing)
private void Dispose(bool disposing)
{
lock (_locko)
{
if (_disposed) return;
_disposed = true;
if (Disposed) return;
Disposed = true;
}
DisposeUnmanagedResources();

View File

@@ -33,7 +33,7 @@ namespace Umbraco.Tests.BootManagers
TestApplicationEventHandler.Reset();
}
/// <summary>
/// test application using a CoreBootManager instance to boot
/// </summary>
@@ -75,7 +75,7 @@ namespace Umbraco.Tests.BootManagers
container.RegisterSingleton<IExamineIndexCollectionAccessor, TestIndexCollectionAccessor>();
}
}
/// <summary>
/// test event handler
/// </summary>
@@ -86,13 +86,13 @@ namespace Umbraco.Tests.BootManagers
Initialized = false;
Starting = false;
Started = false;
Disposed = false;
HasBeenDisposed = false;
}
public static bool Initialized = false;
public static bool Starting = false;
public static bool Started = false;
public static bool Disposed = false;
public static bool Initialized;
public static bool Starting;
public static bool Started;
public static bool HasBeenDisposed;
public void OnApplicationInitialized(UmbracoApplicationBase umbracoApplication, ApplicationContext applicationContext)
{
@@ -111,7 +111,7 @@ namespace Umbraco.Tests.BootManagers
protected override void DisposeResources()
{
Disposed = true;
HasBeenDisposed = true;
}
}
@@ -121,8 +121,8 @@ namespace Umbraco.Tests.BootManagers
using (var app = new TestApp())
{
app.StartApplication(app, new EventArgs());
Assert.IsTrue(TestApplicationEventHandler.Disposed);
Assert.IsTrue(TestApplicationEventHandler.HasBeenDisposed);
}
}

View File

@@ -75,8 +75,7 @@ namespace Umbraco.Tests.Cache.PublishedCache
facadeService.Object,
new WebSecurity(_httpContextFactory.HttpContext, ApplicationContext),
settings,
Enumerable.Empty<IUrlProvider>(),
null);
Enumerable.Empty<IUrlProvider>());
_cache = _umbracoContext.ContentCache;
}

View File

@@ -72,8 +72,7 @@ namespace Umbraco.Tests.PublishedContent
facadeService.Object,
new WebSecurity(httpContext, ApplicationContext),
Mock.Of<IUmbracoSettingsSection>(),
Enumerable.Empty<IUrlProvider>(),
null);
Enumerable.Empty<IUrlProvider>());
Umbraco.Web.Current.SetUmbracoContext(ctx, true);
}

View File

@@ -547,7 +547,7 @@ namespace Umbraco.Tests.Scheduling
runner.Shutdown(false, true);
// check that task has been disposed (timer has been killed, etc)
Assert.IsTrue(task.IsDisposed);
Assert.IsTrue(task.Disposed);
}
}

View File

@@ -43,7 +43,7 @@ namespace Umbraco.Tests.Security
Mock.Of<HttpContextBase>(), appCtx,
Mock.Of<IFacadeService>(),
new WebSecurity(Mock.Of<HttpContextBase>(), appCtx),
Mock.Of<IUmbracoSettingsSection>(), new List<IUrlProvider>(), false);
Mock.Of<IUmbracoSettingsSection>(), new List<IUrlProvider>());
var mgr = new BackOfficeCookieManager(Mock.Of<IUmbracoContextAccessor>(accessor => accessor.UmbracoContext == umbCtx));
@@ -68,7 +68,7 @@ namespace Umbraco.Tests.Security
Mock.Of<HttpContextBase>(), appCtx,
Mock.Of<IFacadeService>(),
new WebSecurity(Mock.Of<HttpContextBase>(), appCtx),
Mock.Of<IUmbracoSettingsSection>(), new List<IUrlProvider>(), false);
Mock.Of<IUmbracoSettingsSection>(), new List<IUrlProvider>());
var mgr = new BackOfficeCookieManager(Mock.Of<IUmbracoContextAccessor>(accessor => accessor.UmbracoContext == umbCtx));

View File

@@ -42,8 +42,7 @@ namespace Umbraco.Tests.Web
Mock.Of<IFacadeService>(),
new WebSecurity(Mock.Of<HttpContextBase>(), appCtx),
Mock.Of<IUmbracoSettingsSection>(),
new List<IUrlProvider>(),
false);
new List<IUrlProvider>());
var r1 = new RouteData();
r1.DataTokens.Add(Core.Constants.Web.UmbracoContextDataToken, umbCtx);
@@ -62,8 +61,7 @@ namespace Umbraco.Tests.Web
Mock.Of<IFacadeService>(),
new WebSecurity(Mock.Of<HttpContextBase>(), appCtx),
Mock.Of<IUmbracoSettingsSection>(),
new List<IUrlProvider>(),
false);
new List<IUrlProvider>());
var r1 = new RouteData();
r1.DataTokens.Add(Core.Constants.Web.UmbracoContextDataToken, umbCtx);
@@ -92,8 +90,7 @@ namespace Umbraco.Tests.Web
Mock.Of<IFacadeService>(),
new WebSecurity(Mock.Of<HttpContextBase>(), appCtx),
Mock.Of<IUmbracoSettingsSection>(),
new List<IUrlProvider>(),
false);
new List<IUrlProvider>());
var httpContext = Mock.Of<HttpContextBase>();

View File

@@ -55,6 +55,7 @@ namespace Umbraco.Web
{
if (UmbracoContextAccessor.UmbracoContext != null && canReplace == false)
throw new InvalidOperationException("Current UmbracoContext can be set only once per request.");
UmbracoContextAccessor.UmbracoContext?.Dispose(); // dispose the one that is being replaced, if any
UmbracoContextAccessor.UmbracoContext = value;
}
}
@@ -62,7 +63,11 @@ namespace Umbraco.Web
// this is because of the weird mixed accessor we're using that can store things in thread-static var
public static void ClearUmbracoContext()
{
UmbracoContextAccessor.UmbracoContext = null;
lock (Locker)
{
UmbracoContextAccessor.UmbracoContext?.Dispose(); // dispose the one that is being cleared, if any
UmbracoContextAccessor.UmbracoContext = null;
}
}
// cannot set - it's set by whatever creates the facade, which should have the accessor injected

View File

@@ -67,7 +67,7 @@ namespace Umbraco.Web.Scheduling
if (_runner.TryAdd(this))
_timer.Change(_periodMilliseconds, 0);
else
Dispose(true);
Dispose();
}
/// <summary>

View File

@@ -67,7 +67,7 @@ namespace Umbraco.Web
// create, assign the singleton, and return
umbracoContext = CreateContext(httpContext, applicationContext, facadeService, webSecurity, umbracoSettings, urlProviders);
Web.Current.SetUmbracoContext(umbracoContext, replaceContext);
Web.Current.SetUmbracoContext(umbracoContext, replaceContext); // will dispose the one that is being replaced
return umbracoContext;
}
@@ -135,9 +135,14 @@ namespace Umbraco.Web
IFacadeService facadeService,
WebSecurity webSecurity)
{
// ensure that this instance is disposed when the request terminates,
// though we *also* ensure this happens in the Umbraco module since the
// UmbracoCOntext is added to the HttpContext items.
// ensure that this instance is disposed when the request terminates, though we *also* ensure
// this happens in the Umbraco module since the UmbracoCOntext is added to the HttpContext items.
//
// also, it *can* be returned by the container with a PerRequest lifetime, meaning that the
// container *could* also try to dispose it.
//
// all in all, this context may be disposed more than once, but DisposableObject ensures that
// it is ok and it will be actually disposed only once.
httpContext.DisposeOnPipelineCompleted(this);
if (httpContext == null) throw new ArgumentNullException(nameof(httpContext));
@@ -357,12 +362,13 @@ namespace Umbraco.Web
}
}
protected override void DisposeResources()
{
// DisposableObject ensures that this runs only once
Security.DisposeIfDisposable();
//If not running in a web ctx, ensure the thread based instance is nulled
// reset - important when running outside of http context
Web.Current.SetUmbracoContext(null, true);
// help caches release resources