yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Updates scopes in execution context to use a Stack so we know which is the top Scope/Context. Fixes disposing things on end request, fixes ensuring orphaned scopes are disposed at end request.

Browse Source
This commit is contained in:
Shannon
2021-03-05 15:27:45 +11:00
parent 7d45a585ee
commit df333ec8cb
13 changed files with 498 additions and 294 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
src/Umbraco.Core/IDisposeOnRequestEnd.cs
View File

@@ -1,10 +1,11 @@
using System;
using System;
namespace Umbraco.Cms.Core
{
/// <summary>
/// Any class implementing this interface that is added to the httpcontext.items keys or values will be disposed of at the end of the request.
/// </summary>
// TODO: Once UmbracoContext no longer needs this (see TODO in UmbracoContext), this should be killed
public interface IDisposeOnRequestEnd : IDisposable
{
}

1
src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs
View File

@@ -77,6 +77,7 @@ namespace Umbraco.Cms.Infrastructure.DependencyInjection
builder.Services.AddUnique<ScopeProvider>(); // implements both IScopeProvider and IScopeAccessor
builder.Services.AddUnique<IScopeProvider>(f => f.GetRequiredService<ScopeProvider>());
builder.Services.AddUnique<IScopeAccessor>(f => f.GetRequiredService<ScopeProvider>());
builder.Services.AddScoped<IHttpScopeReference, HttpScopeReference>();
builder.Services.AddUnique<IJsonSerializer, JsonNetSerializer>();
builder.Services.AddUnique<IConfigurationEditorJsonSerializer, ConfigurationEditorJsonSerializer>();

48
src/Umbraco.Infrastructure/Scoping/HttpScopeReference.cs Normal file
View File

@@ -0,0 +1,48 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
namespace Umbraco.Cms.Core.Scoping
{
/// <summary>
/// Disposed at the end of the request to cleanup any orphaned Scopes.
/// </summary>
/// <remarks>Registered as Scoped in DI (per request)</remarks>
internal class HttpScopeReference : IHttpScopeReference
{
private readonly ScopeProvider _scopeProvider;
private bool _disposedValue;
private bool _registered = false;
public HttpScopeReference(ScopeProvider scopeProvider) => _scopeProvider = scopeProvider;
protected virtual void Dispose(bool disposing)
{
if (!_disposedValue)
{
if (disposing)
{
if (_registered)
{
// dispose the entire chain (if any)
// reset (don't commit by default)
Scope scope;
while ((scope = _scopeProvider.AmbientScope) != null)
{
scope.Reset();
scope.Dispose();
}
}
}
_disposedValue = true;
}
}
public void Dispose() =>
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
Dispose(disposing: true);
public void Register() => _registered = true;
}
}

18
src/Umbraco.Infrastructure/Scoping/IHttpScopeReference.cs Normal file
View File

@@ -0,0 +1,18 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using System;
namespace Umbraco.Cms.Core.Scoping
{
/// <summary>
/// Cleans up orphaned <see cref="IScope"/> references at the end of a request
/// </summary>
public interface IHttpScopeReference : IDisposable
{
/// <summary>
/// Register for cleanup in the request
/// </summary>
void Register();
}
}

131
src/Umbraco.Infrastructure/Scoping/Scope.cs
View File

@@ -7,7 +7,6 @@ using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Infrastructure.Persistence;
using Umbraco.Extensions;
using CoreDebugSettings = Umbraco.Cms.Core.Configuration.Models.CoreDebugSettings;
namespace Umbraco.Cms.Core.Scoping
@@ -71,6 +70,7 @@ namespace Umbraco.Cms.Core.Scoping
Detachable = detachable;
// TODO: The DEBUG_SCOPES flag will not work with recent Stack changes
#if DEBUG_SCOPES
_scopeProvider.RegisterScope(this);
logger.LogDebug("create " + InstanceId.ToString("N").Substring(0, 8));
@@ -188,8 +188,16 @@ namespace Umbraco.Cms.Core.Scoping
{
get
{
if (_callContext) return true;
if (ParentScope != null) return ParentScope.CallContext;
if (_callContext)
{
return true;
}
if (ParentScope != null)
{
return ParentScope.CallContext;
}
return false;
}
set => _callContext = value;
@@ -199,7 +207,11 @@ namespace Umbraco.Cms.Core.Scoping
{
get
{
if (ParentScope != null) return ParentScope.ScopedFileSystems;
if (ParentScope != null)
{
return ParentScope.ScopedFileSystems;
}
return _fscope != null;
}
}
@@ -209,8 +221,16 @@ namespace Umbraco.Cms.Core.Scoping
{
get
{
if (_repositoryCacheMode != RepositoryCacheMode.Unspecified) return _repositoryCacheMode;
if (ParentScope != null) return ParentScope.RepositoryCacheMode;
if (_repositoryCacheMode != RepositoryCacheMode.Unspecified)
{
return _repositoryCacheMode;
}
if (ParentScope != null)
{
return ParentScope.RepositoryCacheMode;
}
return RepositoryCacheMode.Default;
}
}
@@ -220,10 +240,12 @@ namespace Umbraco.Cms.Core.Scoping
{
get
{
if (ParentScope != null) return ParentScope.IsolatedCaches;
if (ParentScope != null)
{
return ParentScope.IsolatedCaches;
}
return _isolatedCaches ?? (_isolatedCaches
= new IsolatedCaches(type => new DeepCloneAppCache(new ObjectCacheAppCache())));
return _isolatedCaches ??= new IsolatedCaches(type => new DeepCloneAppCache(new ObjectCacheAppCache()));
}
}
@@ -249,8 +271,16 @@ namespace Umbraco.Cms.Core.Scoping
{
get
{
if (_isolationLevel != IsolationLevel.Unspecified) return _isolationLevel;
if (ParentScope != null) return ParentScope.IsolationLevel;
if (_isolationLevel != IsolationLevel.Unspecified)
{
return _isolationLevel;
}
if (ParentScope != null)
{
return ParentScope.IsolationLevel;
}
return Database.SqlContext.SqlSyntax.DefaultIsolationLevel;
}
}
@@ -263,14 +293,19 @@ namespace Umbraco.Cms.Core.Scoping
EnsureNotDisposed();
if (_database != null)
{
return _database;
}
if (ParentScope != null)
{
var database = ParentScope.Database;
var currentLevel = database.GetCurrentTransactionIsolationLevel();
IUmbracoDatabase database = ParentScope.Database;
IsolationLevel currentLevel = database.GetCurrentTransactionIsolationLevel();
if (_isolationLevel > IsolationLevel.Unspecified && currentLevel < _isolationLevel)
{
throw new Exception("Scope requires isolation level " + _isolationLevel + ", but got " + currentLevel + " from parent.");
}
return _database = database;
}
@@ -307,8 +342,12 @@ namespace Umbraco.Cms.Core.Scoping
get
{
EnsureNotDisposed();
if (ParentScope != null) return ParentScope.Messages;
return _messages ?? (_messages = new EventMessages());
if (ParentScope != null)
{
return ParentScope.Messages;
}
return _messages ??= new EventMessages();
// TODO: event messages?
// this may be a problem: the messages collection will be cleared at the end of the scope
@@ -334,8 +373,12 @@ namespace Umbraco.Cms.Core.Scoping
get
{
EnsureNotDisposed();
if (ParentScope != null) return ParentScope.Events;
return _eventDispatcher ?? (_eventDispatcher = new QueuingEventDispatcher(_mediaFileSystem));
if (ParentScope != null)
{
return ParentScope.Events;
}
return _eventDispatcher ??= new QueuingEventDispatcher(_mediaFileSystem);
}
}
@@ -343,14 +386,14 @@ namespace Umbraco.Cms.Core.Scoping
public bool Complete()
{
if (_completed.HasValue == false)
{
_completed = true;
}
return _completed.Value;
}
public void Reset()
{
_completed = null;
}
public void Reset() => _completed = null;
public void ChildCompleted(bool? completed)
{
@@ -368,11 +411,18 @@ namespace Umbraco.Cms.Core.Scoping
private void EnsureNotDisposed()
{
// We can't be disposed
if (_disposed)
{
throw new ObjectDisposedException(GetType().FullName);
throw new ObjectDisposedException($"The {nameof(Scope)} ({this.GetDebugInfo()}) is already disposed");
}
// And neither can our ancestors if we're trying to be disposed since
// a child must always be disposed before it's parent.
// This is a safety check, it's actually not entirely possible that a parent can be
// disposed before the child since that will end up with a "not the Ambient" exception.
ParentScope?.EnsureNotDisposed();
// TODO: safer?
//if (Interlocked.CompareExchange(ref _disposed, 1, 0) != 0)
// throw new ObjectDisposedException(GetType().FullName);
@@ -382,7 +432,6 @@ namespace Umbraco.Cms.Core.Scoping
{
EnsureNotDisposed();
if (this != _scopeProvider.AmbientScope)
{
var failedMessage = $"The {nameof(Scope)} {this.GetDebugInfo()} being disposed is not the Ambient {nameof(Scope)} {_scopeProvider.AmbientScope.GetDebugInfo()}. This typically indicates that a child {nameof(Scope)} was not disposed or flowed to a child thread that was not re-joined to the thread that the parent originated (i.e. Task.Run used as a fire and forget task without ExecutionContext.SuppressFlow()).";
@@ -405,8 +454,9 @@ namespace Umbraco.Cms.Core.Scoping
#endif
}
var parent = ParentScope;
_scopeProvider.AmbientScope = parent; // might be null = this is how scopes are removed from context objects
// Replace the Ambient scope with the parent
Scope parent = ParentScope;
_scopeProvider.PopAmbientScope(this); // pop, the parent is on the stack so is now current
#if DEBUG_SCOPES
_scopeProvider.Disposed(this);
@@ -441,9 +491,13 @@ namespace Umbraco.Cms.Core.Scoping
try
{
if (completed)
{
_database.CompleteTransaction();
}
else
{
_database.AbortTransaction();
}
}
catch
{
@@ -456,7 +510,9 @@ namespace Umbraco.Cms.Core.Scoping
_database = null;
if (databaseException)
{
RobustExit(false, true);
}
}
}
@@ -478,14 +534,20 @@ namespace Umbraco.Cms.Core.Scoping
// to ensure we don't leave a scope around, etc
private void RobustExit(bool completed, bool onException)
{
if (onException) completed = false;
if (onException)
{
completed = false;
}
TryFinally(() =>
{
if (_scopeFileSystem == true)
{
if (completed)
{
_fscope.Complete();
}
_fscope.Dispose();
_fscope = null;
}
@@ -493,7 +555,9 @@ namespace Umbraco.Cms.Core.Scoping
{
// deal with events
if (onException == false)
{
_eventDispatcher?.ScopeExit(completed);
}
}, () =>
{
// if *we* created it, then get rid of it
@@ -506,7 +570,7 @@ namespace Umbraco.Cms.Core.Scoping
finally
{
// removes the ambient context (ambient scope already gone)
_scopeProvider.SetAmbient(null);
_scopeProvider.PopAmbientScopeContext();
}
}
}, () =>
@@ -514,7 +578,18 @@ namespace Umbraco.Cms.Core.Scoping
if (Detachable)
{
// get out of the way, restore original
_scopeProvider.SetAmbient(OrigScope, OrigContext);
// TODO: Difficult to know if this is correct since this is all required
// by Deploy which I don't fully understand since there is limited tests on this in the CMS
if (OrigScope != _scopeProvider.AmbientScope)
{
_scopeProvider.PopAmbientScope(_scopeProvider.AmbientScope);
}
if (OrigContext != _scopeProvider.AmbientContext)
{
_scopeProvider.PopAmbientScopeContext();
}
Attached = false;
OrigScope = null;
OrigContext = null;

277
src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs
View File

@@ -8,7 +8,11 @@ using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Infrastructure.Persistence;
using CoreDebugSettings = Umbraco.Cms.Core.Configuration.Models.CoreDebugSettings;
using Umbraco.Extensions;
using System.Collections.Generic;
using System.Collections.Concurrent;
using Umbraco.Cms.Infrastructure.Install.InstallSteps;
// TODO: The DEBUG_SCOPES flag will not work with recent Stack changes
#if DEBUG_SCOPES
using System.Collections.Generic;
using System.Linq;
@@ -40,8 +44,6 @@ namespace Umbraco.Cms.Core.Scoping
_requestCache = requestCache;
// take control of the FileSystems
_fileSystems.IsScoped = () => AmbientScope != null && AmbientScope.ScopedFileSystems;
_scopeReference = new ScopeReference(this);
}
public IUmbracoDatabaseFactory DatabaseFactory { get; }
@@ -53,13 +55,59 @@ namespace Umbraco.Cms.Core.Scoping
private static T GetCallContextObject<T>(string key)
where T : class, IInstanceIdentifiable
{
T obj = CallContext<T>.GetData(key);
if (obj == default(T))
ConcurrentStack<T> stack = CallContext<ConcurrentStack<T>>.GetData(key);
if (stack == null || !stack.TryPeek(out T peek))
{
return null;
}
return obj;
return peek;
}
internal void MoveHttpContextToCallContext<T>(string key)
where T : class, IInstanceIdentifiable
{
var source = (ConcurrentStack<T>)_requestCache.Get(key);
ConcurrentStack<T> stack = CallContext<ConcurrentStack<T>>.GetData(key);
MoveContexts<T>(key, source, stack, (k, v) => CallContext<ConcurrentStack<T>>.SetData(k, v));
}
internal void MoveCallContextToHttpContext<T>(string key)
where T : class, IInstanceIdentifiable
{
ConcurrentStack<T> source = CallContext<ConcurrentStack<T>>.GetData(key);
var stack = (ConcurrentStack<T>)_requestCache.Get(key);
MoveContexts<T>(key, source, stack, (k, v) => _requestCache.Set(k, v));
}
private void MoveContexts<T>(string key, ConcurrentStack<T> source, ConcurrentStack<T> stack, Action<string, ConcurrentStack<T>> setter)
where T : class, IInstanceIdentifiable
{
if (source == null)
{
return;
}
if (stack != null)
{
stack.Clear();
}
else
{
// TODO: This isn't going to copy it back up the execution context chain
stack = new ConcurrentStack<T>();
setter(key, stack);
}
var arr = new T[source.Count];
source.CopyTo(arr, 0);
Array.Reverse(arr);
foreach (T a in arr)
{
stack.Push(a);
}
source.Clear();
}
private static void SetCallContextObject<T>(string key, T value, ILogger<ScopeProvider> logger)
@@ -85,13 +133,14 @@ namespace Umbraco.Cms.Core.Scoping
}
}
#endif
ConcurrentStack<T> stack = CallContext<ConcurrentStack<T>>.GetData(key);
if (value == null)
{
T obj = CallContext<T>.GetData(key);
CallContext<T>.SetData(key, default); // aka remove
if (obj == null)
if (stack != null)
{
return;
stack.TryPop(out _);
}
}
else
@@ -100,8 +149,12 @@ namespace Umbraco.Cms.Core.Scoping
#if DEBUG_SCOPES
logger.LogDebug("AddObject " + value.InstanceId.ToString("N").Substring(0, 8));
#endif
CallContext<T>.SetData(key, value);
if (stack == null)
{
stack = new ConcurrentStack<T>();
}
stack.Push(value);
CallContext<ConcurrentStack<T>>.SetData(key, stack);
}
}
@@ -109,16 +162,16 @@ namespace Umbraco.Cms.Core.Scoping
private T GetHttpContextObject<T>(string key, bool required = true)
where T : class
{
if (!_requestCache.IsAvailable && required)
{
throw new Exception("Request cache is unavailable.");
}
return (T)_requestCache.Get(key);
var stack = (ConcurrentStack<T>)_requestCache.Get(key);
return stack != null && stack.TryPeek(out T peek) ? peek : null;
}
private bool SetHttpContextObject(string key, object value, bool required = true)
private bool SetHttpContextObject<T>(string key, T value, bool required = true)
{
if (!_requestCache.IsAvailable)
{
@@ -149,13 +202,23 @@ namespace Umbraco.Cms.Core.Scoping
}
}
#endif
var stack = (ConcurrentStack<T>)_requestCache.Get(key);
if (value == null)
{
_requestCache.Remove(key);
if (stack != null)
{
stack.TryPop(out _);
}
}
else
{
_requestCache.Set(key, value);
if (stack == null)
{
stack = new ConcurrentStack<T>();
}
stack.Push(value);
_requestCache.Set(key, stack);
}
return true;
@@ -165,10 +228,10 @@ namespace Umbraco.Cms.Core.Scoping
#region Ambient Context
internal static readonly string ContextItemKey = $"{typeof(ScopeProvider).FullName}";
internal static readonly string ContextItemKey = typeof(ScopeProvider).FullName;
/// <summary>
/// Get or set the Ambient (Current) <see cref="IScopeContext"/> for the current execution context.
/// Get the Ambient (Current) <see cref="IScopeContext"/> for the current execution context.
/// </summary>
/// <remarks>
/// The current execution context may be request based (HttpContext) or on a background thread (AsyncLocal)
@@ -181,22 +244,6 @@ namespace Umbraco.Cms.Core.Scoping
IScopeContext value = GetHttpContextObject<IScopeContext>(ContextItemKey, false);
return value ?? GetCallContextObject<IScopeContext>(ContextItemKey);
}
set
{
// clear both
SetHttpContextObject(ContextItemKey, null, false);
SetCallContextObject<IScopeContext>(ContextItemKey, null, _logger);
if (value == null)
{
return;
}
// set http/call context
if (SetHttpContextObject(ContextItemKey, value, false) == false)
{
SetCallContextObject<IScopeContext>(ContextItemKey, value, _logger);
}
}
}
#endregion
@@ -204,11 +251,7 @@ namespace Umbraco.Cms.Core.Scoping
#region Ambient Scope
internal static readonly string ScopeItemKey = typeof(Scope).FullName;
internal static readonly string ScopeRefItemKey = typeof(ScopeReference).FullName;
// only 1 instance which can be disposed and disposed again
private readonly ScopeReference _scopeReference;
IScope IScopeAccessor.AmbientScope => AmbientScope;
/// <summary>
@@ -217,71 +260,75 @@ namespace Umbraco.Cms.Core.Scoping
/// <remarks>
/// The current execution context may be request based (HttpContext) or on a background thread (AsyncLocal)
/// </remarks>
public Scope AmbientScope
{
// try http context, fallback onto call context
// we are casting here because we know its a concrete type
get => (Scope)GetHttpContextObject<IScope>(ScopeItemKey, false)
?? (Scope)GetCallContextObject<IScope>(ScopeItemKey);
set
{
// clear both
SetHttpContextObject(ScopeItemKey, null, false);
SetHttpContextObject(ScopeRefItemKey, null, false);
SetCallContextObject<IScope>(ScopeItemKey, null, _logger);
if (value == null)
{
return;
}
public Scope AmbientScope => (Scope)GetHttpContextObject<IScope>(ScopeItemKey, false) ?? (Scope)GetCallContextObject<IScope>(ScopeItemKey);
// set http/call context
if (value.CallContext == false && SetHttpContextObject(ScopeItemKey, value, false))
{
SetHttpContextObject(ScopeRefItemKey, _scopeReference);
}
else
{
SetCallContextObject<IScope>(ScopeItemKey, value, _logger);
}
public void PopAmbientScope(Scope scope)
{
// pop the stack from all contexts
SetHttpContextObject<IScope>(ScopeItemKey, null, false);
SetCallContextObject<IScope>(ScopeItemKey, null, _logger);
// We need to move the stack to a different context if the parent scope
// is flagged with a different CallContext flag. This is required
// if creating a child scope with callContext: true (thus forcing CallContext)
// when there is actually a current HttpContext available.
// It's weird but is required for Deploy somehow.
bool parentScopeCallContext = (scope.ParentScope?.CallContext ?? false);
if (scope.CallContext && !parentScopeCallContext)
{
MoveCallContextToHttpContext<IScope>(ScopeItemKey);
MoveCallContextToHttpContext<IScopeContext>(ContextItemKey);
}
else if (!scope.CallContext && parentScopeCallContext)
{
MoveHttpContextToCallContext<IScope>(ScopeItemKey);
MoveHttpContextToCallContext<IScopeContext>(ContextItemKey);
}
}
#endregion
/// <summary>
/// Set the Ambient (Current) <see cref="Scope"/> and <see cref="IScopeContext"/> for the current execution context.
/// </summary>
/// <remarks>
/// The current execution context may be request based (HttpContext) or on a background thread (AsyncLocal)
/// </remarks>
public void SetAmbient(Scope scope, IScopeContext context = null)
public void PushAmbientScope(Scope scope)
{
// clear all
SetHttpContextObject(ScopeItemKey, null, false);
SetHttpContextObject(ScopeRefItemKey, null, false);
SetCallContextObject<IScope>(ScopeItemKey, null, _logger);
SetHttpContextObject(ContextItemKey, null, false);
SetCallContextObject<IScopeContext>(ContextItemKey, null, _logger);
if (scope == null)
if (scope is null)
{
if (context != null)
throw new ArgumentNullException(nameof(scope));
}
if (scope.CallContext != false || !SetHttpContextObject<IScope>(ScopeItemKey, scope, false))
{
// In this case, always ensure that the HttpContext items
// is transfered to CallContext and then cleared since we
// may be migrating context with the callContext = true flag.
// This is a weird case when forcing callContext when HttpContext
// is available. Required by Deploy.
if (_requestCache.IsAvailable)
{
throw new ArgumentException("Must be null if scope is null.", nameof(context));
MoveHttpContextToCallContext<IScope>(ScopeItemKey);
MoveHttpContextToCallContext<IScopeContext>(ContextItemKey);
}
return;
SetCallContextObject<IScope>(ScopeItemKey, scope, _logger);
}
}
public void PushAmbientScopeContext(IScopeContext scopeContext)
{
if (scopeContext is null)
{
throw new ArgumentNullException(nameof(scopeContext));
}
if (scope.CallContext == false && SetHttpContextObject(ScopeItemKey, scope, false))
{
SetHttpContextObject(ScopeRefItemKey, _scopeReference);
SetHttpContextObject(ContextItemKey, context);
}
else
{
SetCallContextObject<IScope>(ScopeItemKey, scope, _logger);
SetCallContextObject<IScopeContext>(ContextItemKey, context, _logger);
}
SetHttpContextObject<IScopeContext>(ContextItemKey, scopeContext, false);
SetCallContextObject<IScopeContext>(ContextItemKey, scopeContext, _logger);
}
public void PopAmbientScopeContext()
{
// pop stack from all contexts
SetHttpContextObject<IScopeContext>(ContextItemKey, null, false);
SetCallContextObject<IScopeContext>(ContextItemKey, null, _logger);
}
/// <inheritdoc />
@@ -290,30 +337,35 @@ namespace Umbraco.Cms.Core.Scoping
RepositoryCacheMode repositoryCacheMode = RepositoryCacheMode.Unspecified,
IEventDispatcher eventDispatcher = null,
bool? scopeFileSystems = null)
{
return new Scope(this, _coreDebugSettings, _mediaFileSystem, _loggerFactory.CreateLogger<Scope>(), _fileSystems, true, null, isolationLevel, repositoryCacheMode, eventDispatcher, scopeFileSystems);
}
=> new Scope(this, _coreDebugSettings, _mediaFileSystem, _loggerFactory.CreateLogger<Scope>(), _fileSystems, true, null, isolationLevel, repositoryCacheMode, eventDispatcher, scopeFileSystems);
/// <inheritdoc />
public void AttachScope(IScope other, bool callContext = false)
{
// IScopeProvider.AttachScope works with an IScope
// but here we can only deal with our own Scope class
if (!(other is Scope otherScope))
if (other is not Scope otherScope)
{
throw new ArgumentException("Not a Scope instance.");
}
if (otherScope.Detachable == false)
{
throw new ArgumentException("Not a detachable scope.");
}
if (otherScope.Attached)
{
throw new InvalidOperationException("Already attached.");
}
otherScope.Attached = true;
otherScope.OrigScope = AmbientScope;
otherScope.OrigContext = AmbientContext;
otherScope.CallContext = callContext;
SetAmbient(otherScope, otherScope.Context);
PushAmbientScopeContext(otherScope.Context);
PushAmbientScope(otherScope);
}
/// <inheritdoc />
@@ -330,7 +382,20 @@ namespace Umbraco.Cms.Core.Scoping
throw new InvalidOperationException("Ambient scope is not detachable.");
}
SetAmbient(ambientScope.OrigScope, ambientScope.OrigContext);
PopAmbientScope(ambientScope);
PopAmbientScopeContext();
Scope originalScope = AmbientScope;
if (originalScope != ambientScope.OrigScope)
{
throw new InvalidOperationException($"The detatched scope ({ambientScope.GetDebugInfo()}) does not match the original ({originalScope.GetDebugInfo()})");
}
IScopeContext originalScopeContext = AmbientContext;
if (originalScopeContext != ambientScope.OrigContext)
{
throw new InvalidOperationException($"The detatched scope context does not match the original");
}
ambientScope.OrigScope = null;
ambientScope.OrigContext = null;
ambientScope.Attached = false;
@@ -353,23 +418,19 @@ namespace Umbraco.Cms.Core.Scoping
ScopeContext newContext = ambientContext == null ? new ScopeContext() : null;
var scope = new Scope(this, _coreDebugSettings, _mediaFileSystem, _loggerFactory.CreateLogger<Scope>(), _fileSystems, false, newContext, isolationLevel, repositoryCacheMode, eventDispatcher, scopeFileSystems, callContext, autoComplete);
// assign only if scope creation did not throw!
SetAmbient(scope, newContext ?? ambientContext);
PushAmbientScope(scope);
if (newContext != null)
{
PushAmbientScopeContext(newContext);
}
return scope;
}
var nested = new Scope(this, _coreDebugSettings, _mediaFileSystem, _loggerFactory.CreateLogger<Scope>(), _fileSystems, ambientScope, isolationLevel, repositoryCacheMode, eventDispatcher, scopeFileSystems, callContext, autoComplete);
SetAmbient(nested, AmbientContext);
PushAmbientScope(nested);
return nested;
}
public void Reset()
{
var scope = AmbientScope;
scope?.Reset();
_scopeReference.Dispose();
}
/// <inheritdoc />
public IScopeContext Context => AmbientContext;

33
src/Umbraco.Infrastructure/Scoping/ScopeReference.cs
View File

@@ -1,33 +0,0 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
namespace Umbraco.Cms.Core.Scoping
{
/// <summary>
/// References a scope.
/// </summary>
/// <remarks>Should go into HttpContext to indicate there is also an IScope in context
/// that needs to be disposed at the end of the request (the scope, and the entire scopes
/// chain).</remarks>
internal class ScopeReference : IDisposeOnRequestEnd // implies IDisposable
{
private readonly ScopeProvider _scopeProvider;
public ScopeReference(ScopeProvider scopeProvider)
{
_scopeProvider = scopeProvider;
}
public void Dispose()
{
// dispose the entire chain (if any)
// reset (don't commit by default)
Scope scope;
while ((scope = _scopeProvider.AmbientScope) != null)
{
scope.Reset();
scope.Dispose();
}
}
}
}

86
src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/ScopeTests.cs
View File

@@ -2,6 +2,8 @@
// See LICENSE for more details.
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Castle.Core.Logging;
@@ -9,6 +11,7 @@ using Microsoft.Extensions.Logging;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Infrastructure.Persistence;
using Umbraco.Cms.Tests.Common.Testing;
@@ -25,9 +28,18 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping
[SetUp]
public void SetUp() => Assert.IsNull(ScopeProvider.AmbientScope); // gone
protected override AppCaches GetAppCaches()
{
// Need to have a mockable request cache for tests
var appCaches = new AppCaches(
NoAppCache.Instance,
Mock.Of<IRequestCache>(x => x.IsAvailable == false),
new IsolatedCaches(_ => NoAppCache.Instance));
return appCaches;
}
[Test]
[Ignore("This does not occur in netcore currently because we are not tracking children, nor 'top' Ambient")]
public void GivenChildThread_WhenTheParentDisposes_ThenInvalidOperationExceptionThrows()
public void GivenUncompletedScopeOnChildThread_WhenTheParentCompletes_TheTransactionIsRolledBack()
{
ScopeProvider scopeProvider = ScopeProvider;
@@ -36,25 +48,16 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping
var t = Task.Run(() =>
{
Console.WriteLine("Child Task start: " + scopeProvider.AmbientScope.InstanceId);
// This will evict the parent and set the child
IScope nested = scopeProvider.CreateScope();
Console.WriteLine("Child Task scope created: " + scopeProvider.AmbientScope.InstanceId);
Thread.Sleep(2000); // block, which means the parent stays evicted for a bit
Console.WriteLine("Child Task before dispose: " + scopeProvider.AmbientScope.InstanceId);
nested.Dispose(); // disposing the child will re-add the parent but it's too late, the parent has tried to dispose
Console.WriteLine("Child Task after dispose: " + scopeProvider.AmbientScope.InstanceId);
Thread.Sleep(2000);
nested.Dispose();
});
InvalidOperationException ex = Assert.Throws<InvalidOperationException>(() =>
{
Console.WriteLine("Parent Task disposing: " + scopeProvider.AmbientScope?.InstanceId);
mainScope.Dispose(); // oops! the parent has been evicted at this state
Console.WriteLine("Parent Task disposed: " + scopeProvider.AmbientScope?.InstanceId);
});
Thread.Sleep(1000); // mimic some long running operation that is shorter than the other thread
mainScope.Complete();
Assert.Throws<InvalidOperationException>(() => mainScope.Dispose());
Task.WaitAll(t);
Console.WriteLine(ex);
}
[Test]
@@ -74,7 +77,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping
}
[Test]
public void GivenChildThread_WhenParentDisposedBeforeChild_ChildScopeThrows()
public void GivenChildThread_WhenParentDisposedBeforeChild_ParentScopeThrows()
{
ScopeProvider scopeProvider = ScopeProvider;
@@ -84,25 +87,25 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping
var t = Task.Run(() =>
{
Console.WriteLine("Child Task start: " + scopeProvider.AmbientScope.InstanceId);
// This will push the child scope to the top of the Stack
IScope nested = scopeProvider.CreateScope();
// AsyncLocal has flowed so the AmbientScope here is replaced with nested
// BUT the AmbientScope in the main thread is not replaced which means
// when the main thread's Scope compares this == AmbientScope it will be true,
// similarly when this nested thread's Scope compares this == AmbientScope it will also
// be true. This is why disposing the parent scope succeeds with an exception.
Console.WriteLine("Child Task scope created: " + scopeProvider.AmbientScope.InstanceId);
Thread.Sleep(2000); // block for a bit to ensure the parent task is disposed first
Thread.Sleep(5000); // block for a bit to ensure the parent task is disposed first
Console.WriteLine("Child Task before dispose: " + scopeProvider.AmbientScope.InstanceId);
ObjectDisposedException ex = Assert.Throws<ObjectDisposedException>(() => nested.Dispose());
Console.WriteLine(ex);
nested.Dispose();
Console.WriteLine("Child Task after dispose: " + scopeProvider.AmbientScope.InstanceId);
});
// provide some time for the child thread to start so the ambient context is copied in AsyncLocal
Thread.Sleep(2000);
// now dispose the main without waiting for the child thread to join
Console.WriteLine("Parent Task disposing: " + scopeProvider.AmbientScope.InstanceId);
mainScope.Dispose();
// This will throw because at this stage a child scope has been created which means
// it is the Ambient (top) scope but here we're trying to dispose the non top scope.
Assert.Throws<InvalidOperationException>(() => mainScope.Dispose());
Task.WaitAll(t); // wait for the child to dispose
mainScope.Dispose(); // now it's ok
Console.WriteLine("Parent Task disposed: " + scopeProvider.AmbientScope?.InstanceId);
Task.WaitAll(t);
}
[Test]
@@ -120,12 +123,12 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping
var t = Task.Run(() =>
{
Console.WriteLine("Child Task start: " + scopeProvider.AmbientScope.InstanceId);
IScope nested = scopeProvider.CreateScope();
IScope nested = scopeProvider.CreateScope();
Console.WriteLine("Child Task before dispose: " + scopeProvider.AmbientScope.InstanceId);
nested.Dispose();
Console.WriteLine("Child Task after disposed: " + scopeProvider.AmbientScope.InstanceId);
});
Thread.Sleep(2000); // block for a bit to ensure the child task is disposed first
Console.WriteLine("Parent Task disposing: " + scopeProvider.AmbientScope.InstanceId);
mainScope.Dispose();
@@ -219,6 +222,24 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping
[Test]
public void NestedMigrateScope()
{
// Get the request cache mock and re-configure it to be available and used
var requestCacheDictionary = new Dictionary<string, object>();
IRequestCache requestCache = AppCaches.RequestCache;
var requestCacheMock = Mock.Get(requestCache);
requestCacheMock
.Setup(x => x.IsAvailable)
.Returns(true);
requestCacheMock
.Setup(x => x.Set(It.IsAny<string>(), It.IsAny<object>()))
.Returns((string key, object val) =>
{
requestCacheDictionary.Add(key, val);
return true;
});
requestCacheMock
.Setup(x => x.Get(It.IsAny<string>()))
.Returns((string key) => requestCacheDictionary.TryGetValue(key, out var val) ? val : null);
ScopeProvider scopeProvider = ScopeProvider;
Assert.IsNull(scopeProvider.AmbientScope);
@@ -236,8 +257,9 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping
Assert.AreSame(scope, ((Scope)nested).ParentScope);
// it's moved over to call context
IScope callContextScope = CallContext<IScope>.GetData(ScopeProvider.ScopeItemKey);
ScopeStack<IScope> callContextScope = CallContext<ScopeStack<IScope>>.GetData(ScopeProvider.ScopeItemKey);
Assert.IsNotNull(callContextScope);
Assert.AreEqual(2, callContextScope.Count);
}
// it's naturally back in http context
@@ -595,7 +617,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping
IScope scope = scopeProvider.CreateScope();
IScope nested = scopeProvider.CreateScope();
Assert.IsNotNull(scopeProvider.AmbientScope);
var scopeRef = new ScopeReference(scopeProvider);
var scopeRef = new HttpScopeReference(scopeProvider);
scopeRef.Dispose();
Assert.IsNull(scopeProvider.AmbientScope);
Assert.Throws<ObjectDisposedException>(() =>

10
src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/HttpContextRequestAppCacheTests.cs
View File

@@ -1,13 +1,10 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using System;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core.Cache;
using static Umbraco.Cms.Core.Cache.HttpContextRequestAppCache;
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Cache
{
@@ -22,13 +19,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Cache
base.Setup();
var httpContext = new DefaultHttpContext();
var services = new ServiceCollection();
services.AddScoped<RequestLock>();
var serviceProviderFactory = new DefaultServiceProviderFactory();
IServiceCollection builder = serviceProviderFactory.CreateBuilder(services);
IServiceProvider serviceProvider = serviceProviderFactory.CreateServiceProvider(builder);
httpContext.RequestServices = serviceProvider;
_httpContextAccessor = Mock.Of<IHttpContextAccessor>(x => x.HttpContext == httpContext);
_appCache = new HttpContextRequestAppCache(_httpContextAccessor);
}

142
src/Umbraco.Web.Common/Cache/HttpContextRequestAppCache.cs
View File

@@ -6,6 +6,7 @@ using System.Threading;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.Events;
using Umbraco.Extensions;
@@ -20,7 +21,7 @@ namespace Umbraco.Cms.Core.Cache
/// in order to facilitate the correct locking and releasing allocations.
/// </para>
/// </remarks>
public class HttpContextRequestAppCache : FastDictionaryAppCacheBase, IRequestCache
public class HttpContextRequestAppCache : FastDictionaryAppCacheBase, IRequestCache, IDisposable
{
//private static readonly string s_contextItemsLockKey = $"{typeof(HttpContextRequestAppCache).FullName}::LockEntered";
private readonly IHttpContextAccessor _httpContextAccessor;
@@ -28,8 +29,7 @@ namespace Umbraco.Cms.Core.Cache
/// <summary>
/// Initializes a new instance of the <see cref="HttpRequestAppCache"/> class with a context, for unit tests!
/// </summary>
public HttpContextRequestAppCache(IHttpContextAccessor httpContextAccessor)
=> _httpContextAccessor = httpContextAccessor;
public HttpContextRequestAppCache(IHttpContextAccessor httpContextAccessor) => _httpContextAccessor = httpContextAccessor;
public bool IsAvailable => TryGetContextItems(out _);
@@ -80,14 +80,16 @@ namespace Umbraco.Cms.Core.Cache
//return result.Value;
var value = result.Value; // will not throw (safe lazy)
if (value is SafeLazy.ExceptionHolder eh) eh.Exception.Throw(); // throw once!
if (value is SafeLazy.ExceptionHolder eh)
eh.Exception.Throw(); // throw once!
return value;
}
public bool Set(string key, object value)
{
//no place to cache so just return the callback result
if (!TryGetContextItems(out var items)) return false;
if (!TryGetContextItems(out var items))
return false;
key = GetCacheKey(key);
try
{
@@ -105,7 +107,8 @@ namespace Umbraco.Cms.Core.Cache
public bool Remove(string key)
{
//no place to cache so just return the callback result
if (!TryGetContextItems(out var items)) return false;
if (!TryGetContextItems(out var items))
return false;
key = GetCacheKey(key);
try
{
@@ -126,7 +129,8 @@ namespace Umbraco.Cms.Core.Cache
{
const string prefix = CacheItemPrefix + "-";
if (!TryGetContextItems(out var items)) return Enumerable.Empty<KeyValuePair<object, object>>();
if (!TryGetContextItems(out var items))
return Enumerable.Empty<KeyValuePair<object, object>>();
return items.Cast<KeyValuePair<object, object>>()
.Where(x => x.Key is string s && s.StartsWith(prefix));
@@ -134,7 +138,8 @@ namespace Umbraco.Cms.Core.Cache
protected override void RemoveEntry(string key)
{
if (!TryGetContextItems(out var items)) return;
if (!TryGetContextItems(out var items))
return;
items.Remove(key);
}
@@ -150,51 +155,47 @@ namespace Umbraco.Cms.Core.Cache
protected override void EnterReadLock()
{
if (!TryGetContextItems(out _))
object locker = GetLock();
if (locker == null)
{
return;
}
ReaderWriterLockSlim locker = GetLock();
locker.EnterReadLock();
Monitor.Enter(locker);
}
protected override void EnterWriteLock()
{
if (!TryGetContextItems(out _))
object locker = GetLock();
if (locker == null)
{
return;
}
ReaderWriterLockSlim locker = GetLock();
locker.EnterWriteLock();
Monitor.Enter(locker);
}
protected override void ExitReadLock()
{
if (!TryGetContextItems(out _))
object locker = GetLock();
if (locker == null)
{
return;
}
ReaderWriterLockSlim locker = GetLock();
if (locker.IsReadLockHeld)
if (Monitor.IsEntered(locker))
{
locker.ExitReadLock();
Monitor.Exit(locker);
}
}
protected override void ExitWriteLock()
{
if (!TryGetContextItems(out _))
object locker = GetLock();
if (locker == null)
{
return;
}
ReaderWriterLockSlim locker = GetLock();
if (locker.IsWriteLockHeld)
if (Monitor.IsEntered(locker))
{
locker.ExitWriteLock();
Monitor.Exit(locker);
}
}
@@ -202,12 +203,12 @@ namespace Umbraco.Cms.Core.Cache
public IEnumerator<KeyValuePair<string, object>> GetEnumerator()
{
if (!TryGetContextItems(out var items))
if (!TryGetContextItems(out IDictionary<object, object> items))
{
yield break;
}
foreach (var item in items)
foreach (KeyValuePair<object, object> item in items)
{
yield return new KeyValuePair<string, object>(item.Key.ToString(), item.Value);
}
@@ -219,35 +220,72 @@ namespace Umbraco.Cms.Core.Cache
/// Ensures and returns the current lock
/// </summary>
/// <returns></returns>
private ReaderWriterLockSlim GetLock() => _httpContextAccessor.GetRequiredHttpContext().RequestServices.GetRequiredService<RequestLock>().Locker;
private object GetLock()
{
HttpContext httpContext = _httpContextAccessor.HttpContext;
if (httpContext == null)
{
return null;
}
RequestLock requestLock = httpContext.Features.Get<RequestLock>();
if (requestLock != null)
{
return requestLock.SyncRoot;
}
IFeatureCollection features = httpContext.Features;
HttpResponse response = httpContext.Response;
lock (httpContext)
{
requestLock = new RequestLock();
features.Set(requestLock);
return requestLock.SyncRoot;
}
}
// This is not a typical dispose pattern since this can be called multiple times to dispose
// whatever might be in the current context.
public void Dispose()
{
// need to resolve from request services since IRequestCache is a non DI service and we don't have a logger when created
ILogger<HttpContextRequestAppCache> logger = _httpContextAccessor.HttpContext?.RequestServices?.GetRequiredService<ILogger<HttpContextRequestAppCache>>();
foreach (KeyValuePair<string, object> i in this)
{
// NOTE: All of these will be Lazy<object> since that is how this cache works,
// but we'll include the 2nd check too
if (i.Value is Lazy<object> lazy && lazy.IsValueCreated && lazy.Value is IDisposeOnRequestEnd doer1)
{
try
{
doer1.Dispose();
}
catch (Exception ex)
{
logger.LogError("Could not dispose item with key " + i.Key, ex);
}
}
else if (i.Value is IDisposeOnRequestEnd doer2)
{
try
{
doer2.Dispose();
}
catch (Exception ex)
{
logger.LogError("Could not dispose item with key " + i.Key, ex);
}
}
}
}
/// <summary>
/// Used as Scoped instance to allow locking within a request
/// </summary>
internal class RequestLock : IDisposable
private class RequestLock
{
private bool _disposedValue;
public ReaderWriterLockSlim Locker { get; } = new ReaderWriterLockSlim();
protected virtual void Dispose(bool disposing)
{
if (!_disposedValue)
{
if (disposing)
{
Locker.Dispose();
}
_disposedValue = true;
}
}
public void Dispose()
{
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
Dispose(disposing: true);
}
public object SyncRoot { get; } = new object();
}
}
}

1
src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs
View File

@@ -286,7 +286,6 @@ namespace Umbraco.Extensions
builder.Services.AddSingleton<ContentModelBinder>();
builder.Services.AddScoped<UmbracoHelper>();
builder.Services.AddScoped<RequestLock>();
builder.Services.AddScoped<IBackOfficeSecurity, BackOfficeSecurity>();
builder.Services.AddScoped<IUmbracoWebsiteSecurity, UmbracoWebsiteSecurity>();

39
src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs
View File

@@ -4,12 +4,14 @@ using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Hosting;
using Umbraco.Cms.Core.Logging;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Infrastructure.PublishedCache;
@@ -153,7 +155,7 @@ namespace Umbraco.Cms.Web.Common.Middleware
/// <summary>
/// Any object that is in the HttpContext.Items collection that is IDisposable will get disposed on the end of the request
/// </summary>
private static void DisposeRequestCacheItems(ILogger<UmbracoRequestMiddleware> logger, IRequestCache requestCache, HttpRequest request)
private void DisposeRequestCacheItems(ILogger<UmbracoRequestMiddleware> logger, IRequestCache requestCache, HttpRequest request)
{
// do not process if client-side request
if (request.IsClientSideRequest())
@@ -161,37 +163,16 @@ namespace Umbraco.Cms.Web.Common.Middleware
return;
}
// get a list of keys to dispose
var keys = new HashSet<string>();
foreach (var i in requestCache)
// dispose the request cache at the end of the request
// and it can take care of disposing it's items if there are any
if (requestCache is IDisposable rd)
{
if (i.Value is IDisposeOnRequestEnd || i.Key is IDisposeOnRequestEnd)
{
keys.Add(i.Key);
}
rd.Dispose();
}
// dispose each item and key that was found as disposable.
foreach (var k in keys)
{
try
{
requestCache.Get(k).DisposeIfDisposable();
}
catch (Exception ex)
{
logger.LogError("Could not dispose item with key " + k, ex);
}
try
{
k.DisposeIfDisposable();
}
catch (Exception ex)
{
logger.LogError("Could not dispose item key " + k, ex);
}
}
// ensure this is disposed by DI at the end of the request
IHttpScopeReference httpScopeReference = request.HttpContext.RequestServices.GetRequiredService<IHttpScopeReference>();
httpScopeReference.Register();
}
/// <summary>

3
src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs
View File

@@ -13,6 +13,9 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext
/// <summary>
/// Class that encapsulates Umbraco information of a specific HTTP request
/// </summary>
// TODO: When https://github.com/umbraco/Umbraco-CMS/pull/9916 is merged, remove IDisposeOnRequestEnd
// and just explicitly register the created UmbracoContext with being disposed on end request with
// the HttpContext.Response
public class UmbracoContext : DisposableObjectSlim, IDisposeOnRequestEnd, IUmbracoContext
{
private readonly IHostingEnvironment _hostingEnvironment;