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

Cherry picked - Fix scope leaks caused by database messenger [U4-11207] #2580

Browse Source
This commit is contained in:
Sebastiaan Janssen
2018-04-12 16:23:33 +02:00
parent 3097a46ab3
commit 19241995e8
5 changed files with 97 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/Umbraco.Core/Persistence/PetaPoco.cs
View File

@@ -402,7 +402,7 @@ namespace Umbraco.Core.Persistence
}
// Helper to handle named parameters from object properties
static Regex rxParams = new Regex(@"(?<!@)@\w+", RegexOptions.Compiled);
static readonly Regex rxParams = new Regex(@"(?<!@)@\w+", RegexOptions.Compiled);
public static string ProcessParams(string _sql, object[] args_src, List<object> args_dest)
{
return rxParams.Replace(_sql, m =>
@@ -545,7 +545,7 @@ namespace Umbraco.Core.Persistence
}
// Create a command
static Regex rxParamsPrefix = new Regex(@"(?<!@)@\w+", RegexOptions.Compiled);
static readonly Regex rxParamsPrefix = new Regex(@"(?<!@)@\w+", RegexOptions.Compiled);
public IDbCommand CreateCommand(IDbConnection connection, string sql, params object[] args)
{
// Perform named argument replacements
@@ -666,8 +666,8 @@ namespace Umbraco.Core.Persistence
return ExecuteScalar<T>(sql.SQL, sql.Arguments);
}
Regex rxSelect = new Regex(@"\A\s*(SELECT|EXECUTE|CALL)\s", RegexOptions.Compiled | RegexOptions.Singleline | RegexOptions.IgnoreCase | RegexOptions.Multiline);
Regex rxFrom = new Regex(@"\A\s*FROM\s", RegexOptions.Compiled | RegexOptions.Singleline | RegexOptions.IgnoreCase | RegexOptions.Multiline);
static readonly Regex rxSelect = new Regex(@"\A\s*(SELECT|EXECUTE|CALL)\s", RegexOptions.Compiled | RegexOptions.Singleline | RegexOptions.IgnoreCase | RegexOptions.Multiline);
static readonly Regex rxFrom = new Regex(@"\A\s*FROM\s", RegexOptions.Compiled | RegexOptions.Singleline | RegexOptions.IgnoreCase | RegexOptions.Multiline);
string AddSelectClause<T>(string sql)
{
if (sql.StartsWith(";"))
@@ -701,9 +701,9 @@ namespace Umbraco.Core.Persistence
return Fetch<T>(sql.SQL, sql.Arguments);
}
static Regex rxColumns = new Regex(@"\A\s*SELECT\s+((?:\((?>\((?<depth>)|\)(?<-depth>)|.?)*(?(depth)(?!))\)|.)*?)(?<!,\s+)\bFROM\b", RegexOptions.IgnoreCase | RegexOptions.Multiline | RegexOptions.Singleline | RegexOptions.Compiled);
static Regex rxOrderBy = new Regex(@"\bORDER\s+BY\s+(?:\((?>\((?<depth>)|\)(?<-depth>)|.?)*(?(depth)(?!))\)|[\w\(\)\.])+(?:\s+(?:ASC|DESC))?(?:\s*,\s*(?:\((?>\((?<depth>)|\)(?<-depth>)|.?)*(?(depth)(?!))\)|[\w\(\)\.])+(?:\s+(?:ASC|DESC))?)*", RegexOptions.IgnoreCase | RegexOptions.Multiline | RegexOptions.Singleline | RegexOptions.Compiled);
static Regex rxDistinct = new Regex(@"\ADISTINCT\s", RegexOptions.IgnoreCase | RegexOptions.Multiline | RegexOptions.Singleline | RegexOptions.Compiled);
static readonly Regex rxColumns = new Regex(@"\A\s*SELECT\s+((?:\((?>\((?<depth>)|\)(?<-depth>)|.?)*(?(depth)(?!))\)|.)*?)(?<!,\s+)\bFROM\b", RegexOptions.IgnoreCase | RegexOptions.Multiline | RegexOptions.Singleline | RegexOptions.Compiled);
static readonly Regex rxOrderBy = new Regex(@"\bORDER\s+BY\s+(?:\((?>\((?<depth>)|\)(?<-depth>)|.?)*(?(depth)(?!))\)|[\w\(\)\.])+(?:\s+(?:ASC|DESC))?(?:\s*,\s*(?:\((?>\((?<depth>)|\)(?<-depth>)|.?)*(?(depth)(?!))\)|[\w\(\)\.])+(?:\s+(?:ASC|DESC))?)*", RegexOptions.IgnoreCase | RegexOptions.Multiline | RegexOptions.Singleline | RegexOptions.Compiled);
static readonly Regex rxDistinct = new Regex(@"\ADISTINCT\s", RegexOptions.IgnoreCase | RegexOptions.Multiline | RegexOptions.Singleline | RegexOptions.Compiled);
public static bool SplitSqlForPaging(string sql, out string sqlCount, out string sqlSelectRemoved, out string sqlOrderBy)
{
sqlSelectRemoved = null;
@@ -2546,5 +2546,4 @@ namespace Umbraco.Core.Persistence
}
}
}
}

3
src/Umbraco.Core/Scoping/NoScope.cs
View File

@@ -20,6 +20,7 @@ namespace Umbraco.Core.Scoping
public NoScope(ScopeProvider scopeProvider)
{
_scopeProvider = scopeProvider;
Timestamp = DateTime.Now;
#if DEBUG_SCOPES
_scopeProvider.RegisterScope(this);
#endif
@@ -28,6 +29,8 @@ namespace Umbraco.Core.Scoping
private readonly Guid _instanceId = Guid.NewGuid();
public Guid InstanceId { get { return _instanceId; } }
public DateTime Timestamp { get; }
/// <inheritdoc />
public bool CallContext { get { return false; } }

2
src/Umbraco.Core/Scoping/Scope.cs
View File

@@ -374,7 +374,7 @@ namespace Umbraco.Core.Scoping
}
var parent = ParentScope;
_scopeProvider.AmbientScope = parent;
_scopeProvider.AmbientScope = parent; // might be null = this is how scopes are removed from context objects
#if DEBUG_SCOPES
_scopeProvider.Disposed(this);

57
src/Umbraco.Core/Scoping/ScopeProvider.cs
View File

@@ -2,10 +2,12 @@
using System.Collections;
using System.Collections.Generic;
using System.Data;
using System.Linq;
using System.Runtime.Remoting.Messaging;
using System.Text;
using System.Web;
using Umbraco.Core.Events;
using Umbraco.Core.Logging;
using Umbraco.Core.Persistence;
#if DEBUG_SCOPES
using System.Linq;
@@ -66,22 +68,38 @@ namespace Umbraco.Core.Scoping
// tests, any other things (see https://msdn.microsoft.com/en-us/library/dn458353(v=vs.110).aspx),
// but we don't want to make all of our objects serializable since they are *not* meant to be
// used in cross-AppDomain scenario anyways.
//
// in addition, whatever goes into the logical call context is serialized back and forth any
// time cross-AppDomain code executes, so if we put an "object" there, we'll can *another*
// "object" instance - and so we cannot use a random object as a key.
// time cross-AppDomain code executes, so if we put an "object" there, we'll get *another*
// "object" instance back - and so we cannot use a random object as a key.
//
// so what we do is: we register a guid in the call context, and we keep a table mapping those
// guids to the actual objects. the guid serializes back and forth without causing any issue,
// and we can retrieve the actual objects from the table.
// only issue: how are we supposed to clear the table? we can't, really. objects should take
// care of de-registering themselves from context.
// everything we use does, except the NoScope scope, which just stays there
//
// during tests, NoScope can to into call context... nothing much we can do about it
// so far, the only objects that go into this table are scopes (using ScopeItemKey) and
// scope contexts (using ContextItemKey).
private static readonly object StaticCallContextObjectsLock = new object();
private static readonly Dictionary<Guid, object> StaticCallContextObjects
= new Dictionary<Guid, object>();
// normal scopes and scope contexts take greate care removing themselves when disposed, so it
// is all safe. OTOH the NoScope *CANNOT* remove itself, this is by design, it *WILL* leak and
// there is little (nothing) we can do about it - NoScope exists for backward compatibility
// reasons and relying on it is greatly discouraged.
//
// however... we can *try* at protecting the app against memory leaks, by collecting NoScope
// instances that are too old. if anything actually *need* to retain a NoScope instance for
// a long time, it will break. but that's probably ok. so: the constants below define how
// long a NoScope instance can stay in the table before being removed, and how often we should
// collect the table - and collecting happens anytime SetCallContextObject is invoked
private static readonly TimeSpan StaticCallContextNoScopeLifeSpan = TimeSpan.FromMinutes(30);
private static readonly TimeSpan StaticCallContextCollectPeriod = TimeSpan.FromMinutes(4);
private static DateTime _staticCallContextLastCollect = DateTime.MinValue;
#if DEBUG_SCOPES
public Dictionary<Guid, object> CallContextObjects
{
@@ -156,6 +174,7 @@ namespace Umbraco.Core.Scoping
//Logging.LogHelper.Debug<ScopeProvider>("At:\r\n" + Head(Environment.StackTrace, 24));
#endif
StaticCallContextObjects.Remove(objectKey);
CollectStaticCallContextObjectsLocked();
}
}
else
@@ -171,11 +190,37 @@ namespace Umbraco.Core.Scoping
//Logging.LogHelper.Debug<ScopeProvider>("At:\r\n" + Head(Environment.StackTrace, 24));
#endif
StaticCallContextObjects.Add(objectKey, value);
CollectStaticCallContextObjectsLocked();
}
CallContext.LogicalSetData(key, objectKey);
}
}
private static void CollectStaticCallContextObjectsLocked()
{
// is it time to collect?
var now = DateTime.Now;
if (now - _staticCallContextLastCollect <= StaticCallContextCollectPeriod)
return;
// disable warning: this method is invoked from within a lock
// ReSharper disable InconsistentlySynchronizedField
var threshold = now.Add(-StaticCallContextNoScopeLifeSpan);
var guids = StaticCallContextObjects
.Where(x => x.Value is NoScope noScope && noScope.Timestamp < threshold)
.Select(x => x.Key)
.ToList();
if (guids.Count > 0)
{
LogHelper.Warn<ScopeProvider>($"Collected {guids.Count} NoScope instances from StaticCallContextObjects.");
foreach (var guid in guids)
StaticCallContextObjects.Remove(guid);
}
// ReSharper restore InconsistentlySynchronizedField
_staticCallContextLastCollect = now;
}
// this is for tests exclusively until we have a proper accessor in v8
internal static Func<IDictionary> HttpContextItemsGetter { get; set; }

48
src/Umbraco.Web/BatchedDatabaseServerMessenger.cs
View File

@@ -9,6 +9,7 @@ using Umbraco.Core.Models.Rdbms;
using Umbraco.Core.Sync;
using Umbraco.Web.Routing;
using Umbraco.Core.Logging;
using Umbraco.Core.Scoping;
using Umbraco.Web.Scheduling;
namespace Umbraco.Web
@@ -21,9 +22,12 @@ namespace Umbraco.Web
/// </remarks>
public class BatchedDatabaseServerMessenger : DatabaseServerMessenger
{
private readonly ApplicationContext _appContext;
public BatchedDatabaseServerMessenger(ApplicationContext appContext, bool enableDistCalls, DatabaseServerMessengerOptions options)
: base(appContext, enableDistCalls, options)
{
_appContext = appContext;
Scheduler.Initializing += Scheduler_Initializing;
}
@@ -42,7 +46,7 @@ namespace Umbraco.Web
//start the background task runner for processing instructions
const int delayMilliseconds = 60000;
var instructionProcessingRunner = new BackgroundTaskRunner<IBackgroundTask>("InstructionProcessing", ApplicationContext.ProfilingLogger.Logger);
var instructionProcessingTask = new InstructionProcessing(instructionProcessingRunner, this, delayMilliseconds, Options.ThrottleSeconds * 1000);
var instructionProcessingTask = new InstructionProcessing(instructionProcessingRunner, this, _appContext.ScopeProvider, delayMilliseconds, Options.ThrottleSeconds * 1000);
instructionProcessingRunner.TryAdd(instructionProcessingTask);
e.Add(instructionProcessingTask);
}
@@ -73,18 +77,31 @@ namespace Umbraco.Web
private class InstructionProcessing : RecurringTaskBase
{
private readonly DatabaseServerMessenger _messenger;
private readonly IScopeProvider _scopeProvider;
public InstructionProcessing(IBackgroundTaskRunner<RecurringTaskBase> runner,
DatabaseServerMessenger messenger,
IScopeProvider scopeProvider,
int delayMilliseconds, int periodMilliseconds)
: base(runner, delayMilliseconds, periodMilliseconds)
{
_messenger = messenger;
_scopeProvider = scopeProvider;
}
public override bool PerformRun()
{
_messenger.Sync();
// beware!
// DatabaseServerMessenger uses _appContext.DatabaseContext.Database without creating
// scopes, and since we are running in a background task, there will be no ambient
// scope (as would be the case within a web request), and so we would end up creating
// (and leaking) a NoScope instance, which is bad - better make sure we have a true
// scope here! - see U4-11207
using (var scope = _scopeProvider.CreateScope())
{
_messenger.Sync();
scope.Complete();
}
//return true to repeat
return true;
}
@@ -121,14 +138,17 @@ namespace Umbraco.Web
batch.Clear();
//Write the instructions but only create JSON blobs with a max instruction count equal to MaxProcessingInstructionCount
foreach (var instructionsBatch in instructions.InGroupsOf(Options.MaxProcessingInstructionCount))
using (var scope = _appContext.ScopeProvider.CreateScope())
{
WriteInstructions(instructionsBatch);
foreach (var instructionsBatch in instructions.InGroupsOf(Options.MaxProcessingInstructionCount))
{
WriteInstructions(scope, instructionsBatch);
}
scope.Complete();
}
}
private void WriteInstructions(IEnumerable<RefreshInstruction> instructions)
private void WriteInstructions(IScope scope, IEnumerable<RefreshInstruction> instructions)
{
var dto = new CacheInstructionDto
{
@@ -137,8 +157,7 @@ namespace Umbraco.Web
OriginIdentity = LocalIdentity,
InstructionCount = instructions.Sum(x => x.JsonIdCount)
};
ApplicationContext.DatabaseContext.Database.Insert(dto);
scope.Database.Insert(dto);
}
protected ICollection<RefreshInstructionEnvelope> GetBatch(bool create)
@@ -179,16 +198,19 @@ namespace Umbraco.Web
if (batch == null)
{
//only write the json blob with a maximum count of the MaxProcessingInstructionCount
foreach (var maxBatch in instructions.InGroupsOf(Options.MaxProcessingInstructionCount))
using (var scope = _appContext.ScopeProvider.CreateScope())
{
WriteInstructions(maxBatch);
foreach (var maxBatch in instructions.InGroupsOf(Options.MaxProcessingInstructionCount))
{
WriteInstructions(scope, maxBatch);
}
scope.Complete();
}
}
else
{
batch.Add(new RefreshInstructionEnvelope(servers, refresher, instructions));
}
}
}
}
}
}