Merge pull request #5924 from umbraco/v/bugfix/5035-slot-swap2

Fixes race condition in PublishedSnapshotService and more...
This commit is contained in:
Warren Buckley
2019-07-23 09:18:44 +01:00
committed by GitHub
14 changed files with 101 additions and 63 deletions

View File

@@ -67,31 +67,34 @@ namespace Umbraco.Core
: new NamedSemaphoreReleaser(_semaphore2);
}
public Task<IDisposable> LockAsync()
{
var wait = _semaphore != null
? _semaphore.WaitAsync()
: _semaphore2.WaitOneAsync();
//NOTE: We don't use the "Async" part of this lock at all
//TODO: Remove this and rename this class something like SystemWideLock, then we can re-instate this logic if we ever need an Async lock again
return wait.IsCompleted
? _releaserTask ?? Task.FromResult(CreateReleaser()) // anonymous vs named
: wait.ContinueWith((_, state) => (((AsyncLock) state).CreateReleaser()),
this, CancellationToken.None,
TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
}
//public Task<IDisposable> LockAsync()
//{
// var wait = _semaphore != null
// ? _semaphore.WaitAsync()
// : _semaphore2.WaitOneAsync();
public Task<IDisposable> LockAsync(int millisecondsTimeout)
{
var wait = _semaphore != null
? _semaphore.WaitAsync(millisecondsTimeout)
: _semaphore2.WaitOneAsync(millisecondsTimeout);
// return wait.IsCompleted
// ? _releaserTask ?? Task.FromResult(CreateReleaser()) // anonymous vs named
// : wait.ContinueWith((_, state) => (((AsyncLock) state).CreateReleaser()),
// this, CancellationToken.None,
// TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
//}
return wait.IsCompleted
? _releaserTask ?? Task.FromResult(CreateReleaser()) // anonymous vs named
: wait.ContinueWith((_, state) => (((AsyncLock)state).CreateReleaser()),
this, CancellationToken.None,
TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
}
//public Task<IDisposable> LockAsync(int millisecondsTimeout)
//{
// var wait = _semaphore != null
// ? _semaphore.WaitAsync(millisecondsTimeout)
// : _semaphore2.WaitOneAsync(millisecondsTimeout);
// return wait.IsCompleted
// ? _releaserTask ?? Task.FromResult(CreateReleaser()) // anonymous vs named
// : wait.ContinueWith((_, state) => (((AsyncLock)state).CreateReleaser()),
// this, CancellationToken.None,
// TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
//}
public IDisposable Lock()
{

View File

@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using System.Web.Hosting;
@@ -113,7 +114,7 @@ namespace Umbraco.Core
lock (_locko)
{
_logger.Debug<MainDom>("Signaled {Signaled} ({SignalSource})", _signaled ? "(again)" : string.Empty, source);
_logger.Debug<MainDom>("Signaled ({Signaled}) ({SignalSource})", _signaled ? "again" : "first", source);
if (_signaled) return;
if (_isMainDom == false) return; // probably not needed
_signaled = true;
@@ -171,6 +172,7 @@ namespace Umbraco.Core
// if more than 1 instance reach that point, one will get the lock
// and the other one will timeout, which is accepted
//TODO: This can throw a TimeoutException - in which case should this be in a try/finally to ensure the signal is always reset?
_asyncLocker = _asyncLock.Lock(LockTimeoutMilliseconds);
_isMainDom = true;
@@ -181,6 +183,9 @@ namespace Umbraco.Core
// which is accepted
_signal.Reset();
//WaitOneAsync (ext method) will wait for a signal without blocking the main thread, the waiting is done on a background thread
_signal.WaitOneAsync()
.ContinueWith(_ => OnSignal("signal"));

View File

@@ -139,7 +139,7 @@ namespace Umbraco.Core.Runtime
// there should be none, really - this is here "just in case"
Compose(composition);
// acquire the main domain
// acquire the main domain - if this fails then anything that should be registered with MainDom will not operate
AcquireMainDom(mainDom);
// determine our runtime level
@@ -218,13 +218,13 @@ namespace Umbraco.Core.Runtime
IOHelper.SetRootDirectory(path);
}
private void AcquireMainDom(MainDom mainDom)
private bool AcquireMainDom(MainDom mainDom)
{
using (var timer = ProfilingLogger.DebugDuration<CoreRuntime>("Acquiring MainDom.", "Acquired."))
{
try
{
mainDom.Acquire();
return mainDom.Acquire();
}
catch
{

View File

@@ -97,6 +97,11 @@ namespace Umbraco.Core
/// <param name="request"></param>
internal void EnsureApplicationUrl(HttpRequestBase request = null)
{
//Fixme: This causes problems with site swap on azure because azure pre-warms a site by calling into `localhost` and when it does that
// it changes the URL to `localhost:80` which actually doesn't work for pinging itself, it only works internally in Azure. The ironic part
// about this is that this is here specifically for the slot swap scenario https://issues.umbraco.org/issue/U4-10626
// see U4-10626 - in some cases we want to reset the application url
// (this is a simplified version of what was in 7.x)
// note: should this be optional? is it expensive?

View File

@@ -23,6 +23,8 @@ namespace Umbraco.Core
handle,
(state, timedOut) =>
{
//TODO: We aren't checking if this is timed out
tcs.SetResult(null);
// we take a lock here to make sure the outer method has completed setting the local variable callbackHandle.

View File

@@ -124,7 +124,7 @@ namespace Umbraco.Tests.PublishedContent
_source = new TestDataSource(kits);
// at last, create the complete NuCache snapshot service!
var options = new PublishedSnapshotService.Options { IgnoreLocalDb = true };
var options = new PublishedSnapshotServiceOptions { IgnoreLocalDb = true };
_snapshotService = new PublishedSnapshotService(options,
null,
runtime,

View File

@@ -169,7 +169,7 @@ namespace Umbraco.Tests.PublishedContent
_variationAccesor = new TestVariationContextAccessor();
// at last, create the complete NuCache snapshot service!
var options = new PublishedSnapshotService.Options { IgnoreLocalDb = true };
var options = new PublishedSnapshotServiceOptions { IgnoreLocalDb = true };
_snapshotService = new PublishedSnapshotService(options,
null,
runtime,

View File

@@ -72,7 +72,7 @@ namespace Umbraco.Tests.Scoping
protected override IPublishedSnapshotService CreatePublishedSnapshotService()
{
var options = new PublishedSnapshotService.Options { IgnoreLocalDb = true };
var options = new PublishedSnapshotServiceOptions { IgnoreLocalDb = true };
var publishedSnapshotAccessor = new UmbracoContextPublishedSnapshotAccessor(Umbraco.Web.Composing.Current.UmbracoContextAccessor);
var runtimeStateMock = new Mock<IRuntimeState>();
runtimeStateMock.Setup(x => x.Level).Returns(() => RuntimeLevel.Run);

View File

@@ -42,7 +42,7 @@ namespace Umbraco.Tests.Services
protected override IPublishedSnapshotService CreatePublishedSnapshotService()
{
var options = new PublishedSnapshotService.Options { IgnoreLocalDb = true };
var options = new PublishedSnapshotServiceOptions { IgnoreLocalDb = true };
var publishedSnapshotAccessor = new UmbracoContextPublishedSnapshotAccessor(Umbraco.Web.Composing.Current.UmbracoContextAccessor);
var runtimeStateMock = new Mock<IRuntimeState>();
runtimeStateMock.Setup(x => x.Level).Returns(() => RuntimeLevel.Run);

View File

@@ -15,7 +15,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
// register the NuCache published snapshot service
// must register default options, required in the service ctor
composition.Register(factory => new PublishedSnapshotService.Options());
composition.Register(factory => new PublishedSnapshotServiceOptions());
composition.SetPublishedSnapshotService<PublishedSnapshotService>();
// add the NuCache health check (hidden from type finder)

View File

@@ -30,7 +30,8 @@ using File = System.IO.File;
namespace Umbraco.Web.PublishedCache.NuCache
{
class PublishedSnapshotService : PublishedSnapshotServiceBase
internal class PublishedSnapshotService : PublishedSnapshotServiceBase
{
private readonly ServiceContext _serviceContext;
private readonly IPublishedContentTypeFactory _publishedContentTypeFactory;
@@ -56,7 +57,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
private BPlusTree<int, ContentNodeKit> _localContentDb;
private BPlusTree<int, ContentNodeKit> _localMediaDb;
private readonly bool _localDbExists;
private bool _localDbExists;
// define constant - determines whether to use cache when previewing
// to store eg routes, property converted values, anything - caching
@@ -68,7 +69,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
//private static int _singletonCheck;
public PublishedSnapshotService(Options options, IMainDom mainDom, IRuntimeState runtime,
public PublishedSnapshotService(PublishedSnapshotServiceOptions options, IMainDom mainDom, IRuntimeState runtime,
ServiceContext serviceContext, IPublishedContentTypeFactory publishedContentTypeFactory, IdkMap idkMap,
IPublishedSnapshotAccessor publishedSnapshotAccessor, IVariationContextAccessor variationContextAccessor, ILogger logger, IScopeProvider scopeProvider,
IDocumentRepository documentRepository, IMediaRepository mediaRepository, IMemberRepository memberRepository,
@@ -115,30 +116,36 @@ namespace Umbraco.Web.PublishedCache.NuCache
if (options.IgnoreLocalDb == false)
{
var registered = mainDom.Register(
null,
() =>
{
//"install" phase of MainDom
//this is inside of a lock in MainDom so this is guaranteed to run if MainDom was acquired and guaranteed
//to not run if MainDom wasn't acquired.
//If MainDom was not acquired, then _localContentDb and _localMediaDb will remain null which means this appdomain
//will load in published content via the DB and in that case this appdomain will probably not exist long enough to
//serve more than a page of content.
var path = GetLocalFilesPath();
var localContentDbPath = Path.Combine(path, "NuCache.Content.db");
var localMediaDbPath = Path.Combine(path, "NuCache.Media.db");
_localDbExists = File.Exists(localContentDbPath) && File.Exists(localMediaDbPath);
// if both local databases exist then GetTree will open them, else new databases will be created
_localContentDb = BTree.GetTree(localContentDbPath, _localDbExists);
_localMediaDb = BTree.GetTree(localMediaDbPath, _localDbExists);
},
() =>
{
//"release" phase of MainDom
lock (_storesLock)
{
_contentStore.ReleaseLocalDb();
_contentStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned
_localContentDb = null;
_mediaStore.ReleaseLocalDb();
_mediaStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned
_localMediaDb = null;
}
});
if (registered)
{
var path = GetLocalFilesPath();
var localContentDbPath = Path.Combine(path, "NuCache.Content.db");
var localMediaDbPath = Path.Combine(path, "NuCache.Media.db");
_localDbExists = System.IO.File.Exists(localContentDbPath) && System.IO.File.Exists(localMediaDbPath);
// if both local databases exist then GetTree will open them, else new databases will be created
_localContentDb = BTree.GetTree(localContentDbPath, _localDbExists);
_localMediaDb = BTree.GetTree(localMediaDbPath, _localDbExists);
}
// stores are created with a db so they can write to it, but they do not read from it,
// stores need to be populated, happens in OnResolutionFrozen which uses _localDbExists to
// figure out whether it can read the databases or it should populate them from sql
@@ -251,19 +258,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
base.Dispose();
}
public class Options
{
// disabled: prevents the published snapshot from updating and exposing changes
// or even creating a new published snapshot to see changes, uses old cache = bad
//
//// indicates that the snapshot cache should reuse the application request cache
//// otherwise a new cache object would be created for the snapshot specifically,
//// which is the default - web boot manager uses this to optimize facades
//public bool PublishedSnapshotCacheIsApplicationRequestCache;
public bool IgnoreLocalDb;
}
#endregion
#region Local files

View File

@@ -0,0 +1,28 @@
namespace Umbraco.Web.PublishedCache.NuCache
{
/// <summary>
/// Options class for configuring the <see cref="IPublishedSnapshotService"/>
/// </summary>
public class PublishedSnapshotServiceOptions
{
// disabled: prevents the published snapshot from updating and exposing changes
// or even creating a new published snapshot to see changes, uses old cache = bad
//
//// indicates that the snapshot cache should reuse the application request cache
//// otherwise a new cache object would be created for the snapshot specifically,
//// which is the default - web boot manager uses this to optimize facades
//public bool PublishedSnapshotCacheIsApplicationRequestCache;
/// <summary>
/// If true this disables the persisted local cache files for content and media
/// </summary>
/// <remarks>
/// By default this is false which means umbraco will use locally persisted cache files for reading in all published content and media on application startup.
/// The reason for this is to improve startup times because the alternative to populating the published content and media on application startup is to read
/// these values from the database. In scenarios where sites are relatively small (below a few thousand nodes) reading the content/media from the database to populate
/// the in memory cache isn't that slow and is only marginally slower than reading from the locally persisted cache files.
/// </remarks>
public bool IgnoreLocalDb { get; set; }
}
}

View File

@@ -64,7 +64,7 @@ namespace Umbraco.Web.Scheduling
}
catch (Exception ex)
{
_logger.Error<KeepAlive>(ex, "Failed (at '{UmbracoAppUrl}').", umbracoAppUrl);
_logger.Error<KeepAlive>(ex, "Keep alive failed (at '{UmbracoAppUrl}').", umbracoAppUrl);
}
}

View File

@@ -224,6 +224,7 @@
<Compile Include="Mvc\SurfaceControllerTypeCollectionBuilder.cs" />
<Compile Include="Mvc\ValidateUmbracoFormRouteStringAttribute.cs" />
<Compile Include="Profiling\WebProfilingController.cs" />
<Compile Include="PublishedCache\NuCache\PublishedSnapshotServiceOptions.cs" />
<Compile Include="PublishedCache\NuCache\Snap\GenObj.cs" />
<Compile Include="PublishedCache\NuCache\Snap\GenRef.cs" />
<Compile Include="PublishedCache\NuCache\Snap\LinkedNode.cs" />