Merge branch 'v8/dev' into v8/contrib

This commit is contained in:
Sebastiaan Janssen
2021-03-31 13:01:25 +02:00
28 changed files with 961 additions and 305 deletions

View File

@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Data;
using System.Text;
using Umbraco.Core.Cache;
using Umbraco.Core.Composing;
using Umbraco.Core.Events;
@@ -36,11 +37,10 @@ namespace Umbraco.Core.Scoping
private IEventDispatcher _eventDispatcher;
private object _dictionaryLocker;
// ReadLocks and WriteLocks if we're the outer most scope it's those owned by the entire chain
// If we're a child scope it's those that we have requested.
internal readonly Dictionary<int, int> ReadLocks;
internal readonly Dictionary<int, int> WriteLocks;
private HashSet<int> _readLocks;
private HashSet<int> _writeLocks;
internal Dictionary<Guid, Dictionary<int, int>> ReadLocks;
internal Dictionary<Guid, Dictionary<int, int>> WriteLocks;
// initializes a new scope
private Scope(ScopeProvider scopeProvider,
@@ -67,8 +67,6 @@ namespace Umbraco.Core.Scoping
Detachable = detachable;
_dictionaryLocker = new object();
ReadLocks = new Dictionary<int, int>();
WriteLocks = new Dictionary<int, int>();
#if DEBUG_SCOPES
_scopeProvider.RegisterScope(this);
@@ -345,6 +343,8 @@ namespace Umbraco.Core.Scoping
if (this != _scopeProvider.AmbientScope)
{
var failedMessage = $"The {nameof(Scope)} {this.InstanceId} being disposed is not the Ambient {nameof(Scope)} {(_scopeProvider.AmbientScope?.InstanceId.ToString() ?? "NULL")}. This typically indicates that a child {nameof(Scope)} was not disposed, or flowed to a child thread that was not awaited, or concurrent threads are accessing the same {nameof(Scope)} (Ambient context) which is not supported. If using Task.Run (or similar) as a fire and forget tasks or to run threads in parallel you must suppress execution context flow with ExecutionContext.SuppressFlow() and ExecutionContext.RestoreFlow().";
#if DEBUG_SCOPES
var ambient = _scopeProvider.AmbientScope;
_logger.Debug<Scope>("Dispose error (" + (ambient == null ? "no" : "other") + " ambient)");
@@ -356,24 +356,21 @@ namespace Umbraco.Core.Scoping
+ "- ambient ctor ->\r\n" + ambientInfos.CtorStack + "\r\n"
+ "- dispose ctor ->\r\n" + disposeInfos.CtorStack + "\r\n");
#else
throw new InvalidOperationException("Not the ambient scope.");
throw new InvalidOperationException(failedMessage);
#endif
}
// Decrement the lock counters on the parent if any.
if (ParentScope != null)
ClearLocks(InstanceId);
if (ParentScope is null)
{
lock (_dictionaryLocker)
// We're the parent scope, make sure that locks of all scopes has been cleared
// Since we're only reading we don't have to be in a lock
if (ReadLocks?.Count > 0 || WriteLocks?.Count > 0)
{
foreach (var readLockPair in ReadLocks)
{
DecrementReadLock(readLockPair.Key, readLockPair.Value);
}
foreach (var writeLockPair in WriteLocks)
{
DecrementWriteLock(writeLockPair.Key, writeLockPair.Value);
}
var exception = new InvalidOperationException($"All scopes has not been disposed from parent scope: {InstanceId}, see log for more details.");
_logger.Error<Scope>(exception, GenerateUnclearedScopesLogMessage());
throw exception;
}
}
@@ -396,6 +393,42 @@ namespace Umbraco.Core.Scoping
GC.SuppressFinalize(this);
}
/// <summary>
/// Generates a log message with all scopes that hasn't cleared their locks, including how many, and what locks they have requested.
/// </summary>
/// <returns>Log message.</returns>
private string GenerateUnclearedScopesLogMessage()
{
// Dump the dicts into a message for the locks.
StringBuilder builder = new StringBuilder();
builder.AppendLine($"Lock counters aren't empty, suggesting a scope hasn't been properly disposed, parent id: {InstanceId}");
WriteLockDictionaryToString(ReadLocks, builder, "read locks");
WriteLockDictionaryToString(WriteLocks, builder, "write locks");
return builder.ToString();
}
/// <summary>
/// Writes a locks dictionary to a <see cref="StringBuilder"/> for logging purposes.
/// </summary>
/// <param name="dict">Lock dictionary to report on.</param>
/// <param name="builder">String builder to write to.</param>
/// <param name="dictName">The name to report the dictionary as.</param>
private void WriteLockDictionaryToString(Dictionary<Guid, Dictionary<int, int>> dict, StringBuilder builder, string dictName)
{
if (dict?.Count > 0)
{
builder.AppendLine($"Remaining {dictName}:");
foreach (var instance in dict)
{
builder.AppendLine($"Scope {instance.Key}");
foreach (var lockCounter in instance.Value)
{
builder.AppendLine($"\tLock ID: {lockCounter.Key} - times requested: {lockCounter.Value}");
}
}
}
}
private void DisposeLastScope()
{
// figure out completed
@@ -516,207 +549,157 @@ namespace Umbraco.Core.Scoping
?? (_logUncompletedScopes = Current.Configs.CoreDebug().LogUncompletedScopes)).Value;
/// <summary>
/// Decrements the count of the ReadLocks with a specific lock object identifier we currently hold
/// Increment the counter of a locks dictionary, either ReadLocks or WriteLocks,
/// for a specific scope instance and lock identifier. Must be called within a lock.
/// </summary>
/// <param name="lockId">Lock object identifier to decrement</param>
/// <param name="amountToDecrement">Amount to decrement the lock count with</param>
public void DecrementReadLock(int lockId, int amountToDecrement)
/// <param name="lockId">Lock ID to increment.</param>
/// <param name="instanceId">Instance ID of the scope requesting the lock.</param>
/// <param name="locks">Reference to the dictionary to increment on</param>
private void IncrementLock(int lockId, Guid instanceId, ref Dictionary<Guid, Dictionary<int, int>> locks)
{
// If we aren't the outermost scope, pass it on to the parent.
if (ParentScope != null)
{
ParentScope.DecrementReadLock(lockId, amountToDecrement);
return;
}
// Since we've already checked that we're the parent in the WriteLockInner method, we don't need to check again.
// If it's the very first time a lock has been requested the WriteLocks dict hasn't been instantiated yet.
locks ??= new Dictionary<Guid, Dictionary<int, int>>();
lock (_dictionaryLocker)
// Try and get the dict associated with the scope id.
var locksDictFound = locks.TryGetValue(instanceId, out var locksDict);
if (locksDictFound)
{
ReadLocks[lockId] -= amountToDecrement;
locksDict.TryGetValue(lockId, out var value);
locksDict[lockId] = value + 1;
}
else
{
// The scope hasn't requested a lock yet, so we have to create a dict for it.
locks.Add(instanceId, new Dictionary<int, int>());
locks[instanceId][lockId] = 1;
}
}
/// <summary>
/// Decrements the count of the WriteLocks with a specific lock object identifier we currently hold.
/// Clears all lock counters for a given scope instance, signalling that the scope has been disposed.
/// </summary>
/// <param name="lockId">Lock object identifier to decrement.</param>
/// <param name="amountToDecrement">Amount to decrement the lock count with</param>
public void DecrementWriteLock(int lockId, int amountToDecrement)
/// <param name="instanceId">Instance ID of the scope to clear.</param>
private void ClearLocks(Guid instanceId)
{
// If we aren't the outermost scope, pass it on to the parent.
if (ParentScope != null)
if (ParentScope is not null)
{
ParentScope.DecrementWriteLock(lockId, amountToDecrement);
return;
ParentScope.ClearLocks(instanceId);
}
lock (_dictionaryLocker)
else
{
WriteLocks[lockId] -= amountToDecrement;
}
}
/// <summary>
/// Increment the count of the read locks we've requested
/// </summary>
/// <remarks>
/// This should only be done on child scopes since it's then used to decrement the count later.
/// </remarks>
/// <param name="lockIds"></param>
private void IncrementRequestedReadLock(params int[] lockIds)
{
// We need to keep track of what lockIds we have requested locks for to be able to decrement them.
if (ParentScope != null)
{
foreach (var lockId in lockIds)
lock (_dictionaryLocker)
{
lock (_dictionaryLocker)
{
if (ReadLocks.ContainsKey(lockId))
{
ReadLocks[lockId] += 1;
}
else
{
ReadLocks[lockId] = 1;
}
}
}
}
}
/// <summary>
/// Increment the count of the write locks we've requested
/// </summary>
/// <remarks>
/// This should only be done on child scopes since it's then used to decrement the count later.
/// </remarks>
/// <param name="lockIds"></param>
private void IncrementRequestedWriteLock(params int[] lockIds)
{
// We need to keep track of what lockIds we have requested locks for to be able to decrement them.
if (ParentScope != null)
{
foreach (var lockId in lockIds)
{
lock (_dictionaryLocker)
{
if (WriteLocks.ContainsKey(lockId))
{
WriteLocks[lockId] += 1;
}
else
{
WriteLocks[lockId] = 1;
}
}
ReadLocks?.Remove(instanceId);
WriteLocks?.Remove(instanceId);
}
}
}
/// <inheritdoc />
public void ReadLock(params int[] lockIds)
{
IncrementRequestedReadLock(lockIds);
ReadLockInner(null, lockIds);
}
public void ReadLock(params int[] lockIds) => ReadLockInner(InstanceId, null, lockIds);
/// <inheritdoc />
public void ReadLock(TimeSpan timeout, int lockId)
{
IncrementRequestedReadLock(lockId);
ReadLockInner(timeout, lockId);
}
public void ReadLock(TimeSpan timeout, int lockId) => ReadLockInner(InstanceId, timeout, lockId);
/// <inheritdoc />
public void WriteLock(params int[] lockIds)
{
IncrementRequestedWriteLock(lockIds);
WriteLockInner(null, lockIds);
}
public void WriteLock(params int[] lockIds) => WriteLockInner(InstanceId, null, lockIds);
/// <inheritdoc />
public void WriteLock(TimeSpan timeout, int lockId)
{
IncrementRequestedWriteLock(lockId);
WriteLockInner(timeout, lockId);
}
public void WriteLock(TimeSpan timeout, int lockId) => WriteLockInner(InstanceId, timeout, lockId);
/// <summary>
/// Handles acquiring a read lock, will delegate it to the parent if there are any.
/// </summary>
/// <param name="instanceId">Instance ID of the requesting scope.</param>
/// <param name="timeout">Optional database timeout in milliseconds.</param>
/// <param name="lockIds">Array of lock object identifiers.</param>
internal void ReadLockInner(TimeSpan? timeout = null, params int[] lockIds)
private void ReadLockInner(Guid instanceId, TimeSpan? timeout = null, params int[] lockIds)
{
if (ParentScope != null)
if (ParentScope is not null)
{
// Delegate acquiring the lock to the parent if any.
ParentScope.ReadLockInner(timeout, lockIds);
return;
// If we have a parent we delegate lock creation to parent.
ParentScope.ReadLockInner(instanceId, timeout, lockIds);
}
// If we are the parent, then handle the lock request.
foreach (var lockId in lockIds)
else
{
lock (_dictionaryLocker)
{
// Only acquire the lock if we haven't done so yet.
if (!ReadLocks.ContainsKey(lockId))
{
if (timeout is null)
{
// We want a lock with a custom timeout
ObtainReadLock(lockId);
}
else
{
// We just want an ordinary lock.
ObtainTimoutReadLock(lockId, timeout.Value);
}
// Add the lockId as a key to the dict.
ReadLocks[lockId] = 0;
}
ReadLocks[lockId] += 1;
}
// We are the outermost scope, handle the lock request.
LockInner(instanceId, ref ReadLocks, ref _readLocks, ObtainReadLock, ObtainTimeoutReadLock, timeout, lockIds);
}
}
/// <summary>
/// Handles acquiring a write lock with a specified timeout, will delegate it to the parent if there are any.
/// </summary>
/// <param name="instanceId">Instance ID of the requesting scope.</param>
/// <param name="timeout">Optional database timeout in milliseconds.</param>
/// <param name="lockIds">Array of lock object identifiers.</param>
internal void WriteLockInner(TimeSpan? timeout = null, params int[] lockIds)
private void WriteLockInner(Guid instanceId, TimeSpan? timeout = null, params int[] lockIds)
{
if (ParentScope != null)
if (ParentScope is not null)
{
// If we have a parent we delegate lock creation to parent.
ParentScope.WriteLockInner(timeout, lockIds);
return;
ParentScope.WriteLockInner(instanceId, timeout, lockIds);
}
foreach (var lockId in lockIds)
else
{
lock (_dictionaryLocker)
{
// Only acquire lock if we haven't yet (WriteLocks not containing the key)
if (!WriteLocks.ContainsKey(lockId))
{
if (timeout is null)
{
ObtainWriteLock(lockId);
}
else
{
ObtainTimeoutWriteLock(lockId, timeout.Value);
}
// Add the lockId as a key to the dict.
WriteLocks[lockId] = 0;
}
// We are the outermost scope, handle the lock request.
LockInner(instanceId, ref WriteLocks, ref _writeLocks, ObtainWriteLock, ObtainTimeoutWriteLock, timeout, lockIds);
}
}
// Increment count of the lock by 1.
WriteLocks[lockId] += 1;
/// <summary>
/// Handles acquiring a lock, this should only be called from the outermost scope.
/// </summary>
/// <param name="instanceId">Instance ID of the scope requesting the lock.</param>
/// <param name="locks">Reference to the applicable locks dictionary (ReadLocks or WriteLocks).</param>
/// <param name="locksSet">Reference to the applicable locks hashset (_readLocks or _writeLocks).</param>
/// <param name="obtainLock">Delegate used to request the lock from the database without a timeout.</param>
/// <param name="obtainLockTimeout">Delegate used to request the lock from the database with a timeout.</param>
/// <param name="timeout">Optional timeout parameter to specify a timeout.</param>
/// <param name="lockIds">Lock identifiers to lock on.</param>
private void LockInner(Guid instanceId, ref Dictionary<Guid, Dictionary<int, int>> locks, ref HashSet<int> locksSet,
Action<int> obtainLock, Action<int, TimeSpan> obtainLockTimeout, TimeSpan? timeout = null,
params int[] lockIds)
{
lock (_dictionaryLocker)
{
locksSet ??= new HashSet<int>();
foreach (var lockId in lockIds)
{
// Only acquire the lock if we haven't done so yet.
if (!locksSet.Contains(lockId))
{
IncrementLock(lockId, instanceId, ref locks);
locksSet.Add(lockId);
try
{
if (timeout is null)
{
// We just want an ordinary lock.
obtainLock(lockId);
}
else
{
// We want a lock with a custom timeout
obtainLockTimeout(lockId, timeout.Value);
}
}
catch
{
// Something went wrong and we didn't get the lock
// Since we at this point have determined that we haven't got any lock with an ID of LockID, it's safe to completely remove it instead of decrementing.
locks[instanceId].Remove(lockId);
// It needs to be removed from the HashSet as well, because that's how we determine to acquire a lock.
locksSet.Remove(lockId);
throw;
}
}
else
{
// We already have a lock, but need to update the dictionary for debugging purposes.
IncrementLock(lockId, instanceId, ref locks);
}
}
}
}
@@ -735,10 +718,10 @@ namespace Umbraco.Core.Scoping
/// </summary>
/// <param name="lockId">Lock object identifier to lock.</param>
/// <param name="timeout">TimeSpan specifying the timout period.</param>
private void ObtainTimoutReadLock(int lockId, TimeSpan timeout)
private void ObtainTimeoutReadLock(int lockId, TimeSpan timeout)
{
var syntax2 = Database.SqlContext.SqlSyntax as ISqlSyntaxProvider2;
if (syntax2 == null)
if (syntax2 is null)
{
throw new InvalidOperationException($"{Database.SqlContext.SqlSyntax.GetType()} is not of type {typeof(ISqlSyntaxProvider2)}");
}
@@ -763,7 +746,7 @@ namespace Umbraco.Core.Scoping
private void ObtainTimeoutWriteLock(int lockId, TimeSpan timeout)
{
var syntax2 = Database.SqlContext.SqlSyntax as ISqlSyntaxProvider2;
if (syntax2 == null)
if (syntax2 is null)
{
throw new InvalidOperationException($"{Database.SqlContext.SqlSyntax.GetType()} is not of type {typeof(ISqlSyntaxProvider2)}");
}

View File

@@ -1,6 +1,7 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Runtime.Remoting.Messaging;
using System.Web;
@@ -240,6 +241,9 @@ namespace Umbraco.Core.Scoping
var value = GetHttpContextObject<ScopeContext>(ContextItemKey, false);
return value ?? GetCallContextObject<ScopeContext>(ContextItemKey);
}
[Obsolete("This setter is not used and will be removed in future versions")]
[EditorBrowsable(EditorBrowsableState.Never)]
set
{
// clear both