U4-9322 - fix scope leaks

This commit is contained in:
Stephan
2017-01-18 10:36:11 +01:00
parent 7495cd3f2c
commit 1f636d05d1
7 changed files with 146 additions and 27 deletions

View File

@@ -46,7 +46,7 @@ namespace Umbraco.Core.Persistence
public TResult WithReadLocked<TResult>(Func<LockedRepository<TRepository>, TResult> func, bool autoCommit = true)
{
var uow = _uowProvider.GetUnitOfWork();
using (var uow = _uowProvider.GetUnitOfWork())
using (var transaction = uow.Database.GetTransaction(IsolationLevel.RepeatableRead))
{
foreach (var lockId in _readLockIds)
@@ -65,7 +65,9 @@ namespace Umbraco.Core.Persistence
public void WithWriteLocked(Action<LockedRepository<TRepository>> action, bool autoCommit = true)
{
var uow = _uowProvider.GetUnitOfWork();
// must use the uow to ensure it's disposed if GetTransaction fails!
using (var uow = _uowProvider.GetUnitOfWork())
using (var transaction = uow.Database.GetTransaction(IsolationLevel.RepeatableRead))
{
foreach (var lockId in _writeLockIds)
@@ -79,11 +81,25 @@ namespace Umbraco.Core.Persistence
transaction.Complete();
}
}
// fixme
// the change above ensures that scopes are properly disposed
// TODO apply to all methods
// now how can we manage the isolation level?
//uow.ScopeIsolationLevel = IsolationLevel.RepeatableRead; // might throw when creating the scope
// getTransaction here throws because of different levels
// but the exception gets eaten because on the way out, disposing the _outer_ scope fails
// so... how shall we figure it out?!
// - we should be able to set the isolation level on the uow scope
// - should we be able to dispose parent scopes and then what happens? NO!
//
// if we can create scopes with isolation levels what happens when we nest scopes?
// note: this is a *very* special use case
}
public TResult WithWriteLocked<TResult>(Func<LockedRepository<TRepository>, TResult> func, bool autoCommit = true)
{
var uow = _uowProvider.GetUnitOfWork();
using (var uow = _uowProvider.GetUnitOfWork())
using (var transaction = uow.Database.GetTransaction(IsolationLevel.RepeatableRead))
{
foreach (var lockId in _writeLockIds)

View File

@@ -24,5 +24,9 @@ namespace Umbraco.Core.Scoping
/// Completes the scope.
/// </summary>
void Complete();
#if DEBUG_SCOPES
Guid InstanceId { get; }
#endif
}
}

View File

@@ -1,4 +1,6 @@
namespace Umbraco.Core.Scoping
using System.Collections.Generic;
namespace Umbraco.Core.Scoping
{
/// <summary>
/// Provides scopes.
@@ -43,5 +45,9 @@
/// <para>Only a scope previously attached by <see cref="AttachScope"/> can be detached.</para>
/// </remarks>
IScope DetachScope();
#if DEBUG_SCOPES
IEnumerable<ScopeInfo> ScopeInfos { get; }
#endif
}
}

View File

@@ -19,8 +19,16 @@ namespace Umbraco.Core.Scoping
public NoScope(ScopeProvider scopeProvider)
{
_scopeProvider = scopeProvider;
#if DEBUG_SCOPES
_scopeProvider.Register(this);
#endif
}
#if DEBUG_SCOPES
private readonly Guid _instanceId = Guid.NewGuid();
public Guid InstanceId { get { return _instanceId; } }
#endif
/// <inheritdoc />
public UmbracoDatabase Database
{
@@ -78,6 +86,10 @@ namespace Umbraco.Core.Scoping
if (this != _scopeProvider.AmbientScope)
throw new InvalidOperationException("Not the ambient scope.");
#if DEBUG_SCOPES
_scopeProvider.Disposed(this);
#endif
if (_database != null)
_database.Dispose();

View File

@@ -23,6 +23,9 @@ namespace Umbraco.Core.Scoping
{
_scopeProvider = scopeProvider;
Detachable = detachable;
#if DEBUG_SCOPES
_scopeProvider.Register(this);
#endif
}
// initializes a new scope in a nested scopes chain, with its parent
@@ -45,6 +48,11 @@ namespace Umbraco.Core.Scoping
throw new Exception("NoScope instance is not free.");
}
#if DEBUG_SCOPES
private readonly Guid _instanceId = Guid.NewGuid();
public Guid InstanceId { get { return _instanceId; } }
#endif
// a value indicating whether the scope is detachable
// ie whether it was created by CreateDetachedScope
public bool Detachable { get; private set; }
@@ -139,6 +147,10 @@ namespace Umbraco.Core.Scoping
if (this != _scopeProvider.AmbientScope)
throw new InvalidOperationException("Not the ambient scope.");
#if DEBUG_SCOPES
_scopeProvider.Disposed(this);
#endif
var parent = ParentScope;
_scopeProvider.AmbientScope = parent;

View File

@@ -1,4 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.Remoting.Messaging;
using System.Web;
using Umbraco.Core.Persistence;
@@ -46,6 +48,12 @@ namespace Umbraco.Core.Scoping
get { return (IScope) CallContext.LogicalGetData(ItemKey); }
set
{
#if DEBUG_SCOPES
var ambient = (IScope)CallContext.LogicalGetData(ItemKey);
if (ambient != null) RegisterContext(ambient, null);
if (value != null)
RegisterContext(value, "lcc");
#endif
if (value == null) CallContext.FreeNamedDataSlot(ItemKey);
else CallContext.LogicalSetData(ItemKey, value);
}
@@ -56,6 +64,12 @@ namespace Umbraco.Core.Scoping
get { return (IScope) HttpContext.Current.Items[ItemKey]; }
set
{
#if DEBUG_SCOPES
var ambient = (IScope) HttpContext.Current.Items[ItemKey];
if (ambient != null) RegisterContext(ambient, null);
if (value != null)
RegisterContext(value, "http");
#endif
if (value == null)
{
HttpContext.Current.Items.Remove(ItemKey);
@@ -152,6 +166,9 @@ namespace Umbraco.Core.Scoping
var noScope = ambient as NoScope;
if (noScope != null)
{
#if DEBUG_SCOPES
Disposed(noScope);
#endif
// peta poco nulls the shared connection after each command unless there's a trx
var database = noScope.DatabaseOrNull;
if (database != null && database.InTransaction)
@@ -182,29 +199,82 @@ namespace Umbraco.Core.Scoping
// so we can track leaks
// helps identifying when non-httpContext scopes are created by logging the stack trace
private void LogCallContextStack()
{
var trace = Environment.StackTrace;
if (trace.IndexOf("ScheduledPublishing") > 0)
LogHelper.Debug<ScopeProvider>("CallContext: Scheduled Publishing");
else if (trace.IndexOf("TouchServerTask") > 0)
LogHelper.Debug<ScopeProvider>("CallContext: Server Registration");
else if (trace.IndexOf("LogScrubber") > 0)
LogHelper.Debug<ScopeProvider>("CallContext: Log Scrubber");
else
LogHelper.Debug<ScopeProvider>("CallContext: " + Environment.StackTrace);
}
private readonly List<IScope> _scopes = new List<IScope>();
//private void LogCallContextStack()
//{
// var trace = Environment.StackTrace;
// if (trace.IndexOf("ScheduledPublishing") > 0)
// LogHelper.Debug<ScopeProvider>("CallContext: Scheduled Publishing");
// else if (trace.IndexOf("TouchServerTask") > 0)
// LogHelper.Debug<ScopeProvider>("CallContext: Server Registration");
// else if (trace.IndexOf("LogScrubber") > 0)
// LogHelper.Debug<ScopeProvider>("CallContext: Log Scrubber");
// else
// LogHelper.Debug<ScopeProvider>("CallContext: " + Environment.StackTrace);
//}
// helps identifying scope leaks by keeping track of all instances
public List<IScope> Scopes { get { return _scopes; } }
public static readonly List<ScopeInfo> StaticScopeInfos = new List<ScopeInfo>();
private static void Log(string message, UmbracoDatabase database)
public IEnumerable<ScopeInfo> ScopeInfos { get { return StaticScopeInfos; } }
//private static void Log(string message, UmbracoDatabase database)
//{
// LogHelper.Debug<ScopeProvider>(message + " (" + (database == null ? "" : database.InstanceSid) + ").");
//}
public void Register(IScope scope)
{
LogHelper.Debug<ScopeProvider>(message + " (" + (database == null ? "" : database.InstanceSid) + ").");
if (StaticScopeInfos.Any(x => x.Scope == scope)) throw new Exception();
StaticScopeInfos.Add(new ScopeInfo(scope, Environment.StackTrace));
}
public static void RegisterContext(IScope scope, string context)
{
var info = StaticScopeInfos.FirstOrDefault(x => x.Scope == scope);
if (info == null)
{
if (context == null) return;
throw new Exception();
}
if (context == null) info.NullStack = Environment.StackTrace;
info.Context = context;
}
public void Disposed(IScope scope)
{
var info = StaticScopeInfos.FirstOrDefault(x => x.Scope == scope);
if (info != null)
{
// enable this by default
StaticScopeInfos.Remove(info);
// instead, enable this to keep *all* scopes
// beware, there can be a lot of scopes!
//info.Disposed = true;
//info.DisposedStack = Environment.StackTrace;
}
}
#endif
}
#if DEBUG_SCOPES
public class ScopeInfo
{
public ScopeInfo(IScope scope, string ctorStack)
{
Scope = scope;
Created = DateTime.Now;
CtorStack = ctorStack;
}
public IScope Scope { get; private set; }
public Guid Parent { get { return (Scope is NoScope || ((Scope) Scope).ParentScope == null) ? Guid.Empty : ((Scope) Scope).ParentScope.InstanceId; } }
public DateTime Created { get; private set; }
public bool Disposed { get; set; }
public string Context { get; set; }
public string CtorStack { get; private set; } // the stacktrace of the scope ctor
public string DisposedStack { get; set; } // the stacktrace when disposed
public string NullStack { get; set; } // the stacktrace when context went null
}
#endif
}

View File

@@ -8,6 +8,7 @@ using System.Web.Hosting;
using log4net;
using Umbraco.Core.Logging;
using Umbraco.Core.ObjectResolution;
using Umbraco.Core.Scoping;
namespace Umbraco.Core
{
@@ -60,12 +61,10 @@ namespace Umbraco.Core
//And now we can dispose of our startup handlers - save some memory
ApplicationEventsResolver.Current.Dispose();
// after Umbraco has started there is a database in "context" and that context is
// after Umbraco has started there is a scope in "context" and that context is
// going to stay there and never get destroyed nor reused, so we have to ensure that
// the database is disposed (which will auto-remove it from context).
var database = ApplicationContext.Current.DatabaseContext.Database;
if (database != null) // never to happen... unless in weird tests
ApplicationContext.Current.DatabaseContext.Database.Dispose();
// the scope is disposed (along with database etc) - reset it all entirely
((ScopeProvider) ApplicationContext.Current.ScopeProvider).Reset();
}
/// <summary>