Skip to content

Commit

Permalink
Fix dead lock with TypeLoader
Browse files Browse the repository at this point in the history
  • Loading branch information
@Shazwazza @mikecp
Shazwazza authored and mikecp committed Mar 15, 2021
1 parent 5ee0f31 commit b7c1ad2
Showing 1 changed file with 81 additions and 34 deletions.
115 changes: 81 additions & 34 deletions src/Umbraco.Core/Composing/TypeLoader.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
Expand Down Expand Up @@ -45,7 +45,7 @@ public class TypeLoader
private IEnumerable<Assembly> _assemblies;
private bool _reportedChange;
private readonly string _localTempPath;
private string _fileBasePath;
private readonly Lazy<string> _fileBasePath;

/// <summary>
/// Initializes a new instance of the <see cref="TypeLoader"/> class.
Expand All @@ -70,6 +70,8 @@ internal TypeLoader(IAppPolicyCache runtimeCache, string localTempPath, IProfili
_localTempPath = localTempPath;
_logger = logger ?? throw new ArgumentNullException(nameof(logger));

_fileBasePath = new Lazy<string>(GetFileBasePath);

if (detectChanges)
{
//first check if the cached hash is string.Empty, if it is then we need
Expand Down Expand Up @@ -160,7 +162,8 @@ internal string CachedAssembliesHash
return _cachedAssembliesHash;

var typesHashFilePath = GetTypesHashFilePath();
if (!File.Exists(typesHashFilePath)) return string.Empty;
if (!File.Exists(typesHashFilePath))
return string.Empty;

var hash = File.ReadAllText(typesHashFilePath, Encoding.UTF8);

Expand Down Expand Up @@ -339,19 +342,31 @@ internal Dictionary<Tuple<string, string>, IEnumerable<string>> ReadCache()

var typesListFilePath = GetTypesListFilePath();
if (File.Exists(typesListFilePath) == false)
{
return cache;
}

using (var stream = GetFileStream(typesListFilePath, FileMode.Open, FileAccess.Read, FileShare.Read, ListFileOpenReadTimeout))
using (var reader = new StreamReader(stream))
{
while (true)
{
var baseType = reader.ReadLine();
if (baseType == null) return cache; // exit
if (baseType.StartsWith("<")) break; // old xml
if (baseType == null)
{
return cache; // exit
}

if (baseType.StartsWith("<"))
{
break; // old xml
}

var attributeType = reader.ReadLine();
if (attributeType == null) break;
if (attributeType == null)
{
break;
}

var types = new List<string>();
while (true)
Expand All @@ -370,7 +385,10 @@ internal Dictionary<Tuple<string, string>, IEnumerable<string>> ReadCache()
types.Add(type);
}

if (types == null) break;
if (types == null)
{
break;
}
}
}

Expand All @@ -379,28 +397,31 @@ internal Dictionary<Tuple<string, string>, IEnumerable<string>> ReadCache()
}

// internal for tests
internal string GetTypesListFilePath() => GetFileBasePath() + ".list";
internal string GetTypesListFilePath() => _fileBasePath.Value + ".list";

private string GetTypesHashFilePath() => GetFileBasePath() + ".hash";
private string GetTypesHashFilePath() => _fileBasePath.Value + ".hash";

/// <summary>
/// Used to produce the Lazy value of _fileBasePath
/// </summary>
/// <returns></returns>
private string GetFileBasePath()
{
lock (_locko)
{
if (_fileBasePath != null)
return _fileBasePath;

_fileBasePath = Path.Combine(_localTempPath, "TypesCache", "umbraco-types." + NetworkHelper.FileSafeMachineName);
var fileBasePath = Path.Combine(_localTempPath, "TypesCache", "umbraco-types." + NetworkHelper.FileSafeMachineName);

// ensure that the folder exists
var directory = Path.GetDirectoryName(_fileBasePath);
if (directory == null)
throw new InvalidOperationException($"Could not determine folder for path \"{_fileBasePath}\".");
if (Directory.Exists(directory) == false)
Directory.CreateDirectory(directory);
// ensure that the folder exists
var directory = Path.GetDirectoryName(fileBasePath);
if (directory == null)
{
throw new InvalidOperationException($"Could not determine folder for path \"{fileBasePath}\".");
}

return _fileBasePath;
if (Directory.Exists(directory) == false)
{
Directory.CreateDirectory(directory);
}

return fileBasePath;
}

// internal for tests
Expand All @@ -416,7 +437,10 @@ internal void WriteCache()
writer.WriteLine(typeList.BaseType == null ? string.Empty : typeList.BaseType.FullName);
writer.WriteLine(typeList.AttributeType == null ? string.Empty : typeList.AttributeType.FullName);
foreach (var type in typeList.Types)
{
writer.WriteLine(type.AssemblyQualifiedName);
}

writer.WriteLine();
}
}
Expand All @@ -434,16 +458,22 @@ void TimerRelease(object o)
WriteCache();
}
catch { /* bah - just don't die */ }
if (!_timing) _timer = null;
if (!_timing)
_timer = null;
}
}

lock (_timerLock)
{
if (_timer == null)
{
_timer = new Timer(TimerRelease, null, ListFileWriteThrottle, Timeout.Infinite);
}
else
{
_timer.Change(ListFileWriteThrottle, Timeout.Infinite);
}

_timing = true;
}
}
Expand Down Expand Up @@ -476,7 +506,9 @@ private Stream GetFileStream(string path, FileMode fileMode, FileAccess fileAcce
catch
{
if (--attempts == 0)
{
throw;
}

_logger.Debug<TypeLoader,string,int,int>("Attempted to get filestream for file {Path} failed, {NumberOfAttempts} attempts left, pausing for {PauseMilliseconds} milliseconds", path, attempts, pauseMilliseconds);
Thread.Sleep(pauseMilliseconds);
Expand Down Expand Up @@ -543,7 +575,8 @@ public IEnumerable<Attribute> GetAssemblyAttributes()
/// <exception cref="ArgumentNullException">attributeTypes</exception>
public IEnumerable<Attribute> GetAssemblyAttributes(params Type[] attributeTypes)
{
if (attributeTypes == null) throw new ArgumentNullException(nameof(attributeTypes));
if (attributeTypes == null)
throw new ArgumentNullException(nameof(attributeTypes));

return AssembliesToScan.SelectMany(a => attributeTypes.SelectMany(at => a.GetCustomAttributes(at))).ToList();
}
Expand All @@ -563,7 +596,9 @@ public IEnumerable<Attribute> GetAssemblyAttributes(params Type[] attributeTypes
public IEnumerable<Type> GetTypes<T>(bool cache = true, IEnumerable<Assembly> specificAssemblies = null)
{
if (_logger == null)
{
throw new InvalidOperationException("Cannot get types from a test/blank type loader.");
}

// do not cache anything from specific assemblies
cache &= specificAssemblies == null;
Expand All @@ -583,7 +618,7 @@ public IEnumerable<Type> GetTypes<T>(bool cache = true, IEnumerable<Assembly> sp

// get IDiscoverable and always cache
var discovered = GetTypesInternal(
typeof (IDiscoverable), null,
typeof(IDiscoverable), null,
() => TypeFinder.FindClassesOfType<IDiscoverable>(AssembliesToScan),
"scanning assemblies",
true);
Expand All @@ -594,9 +629,9 @@ public IEnumerable<Type> GetTypes<T>(bool cache = true, IEnumerable<Assembly> sp

// filter the cached discovered types (and maybe cache the result)
return GetTypesInternal(
typeof (T), null,
typeof(T), null,
() => discovered
.Where(x => typeof (T).IsAssignableFrom(x)),
.Where(x => typeof(T).IsAssignableFrom(x)),
"filtering IDiscoverable",
cache);
}
Expand All @@ -614,7 +649,9 @@ public IEnumerable<Type> GetTypesWithAttribute<T, TAttribute>(bool cache = true,
where TAttribute : Attribute
{
if (_logger == null)
{
throw new InvalidOperationException("Cannot get types from a test/blank type loader.");
}

// do not cache anything from specific assemblies
cache &= specificAssemblies == null;
Expand All @@ -633,7 +670,7 @@ public IEnumerable<Type> GetTypesWithAttribute<T, TAttribute>(bool cache = true,

// get IDiscoverable and always cache
var discovered = GetTypesInternal(
typeof (IDiscoverable), null,
typeof(IDiscoverable), null,
() => TypeFinder.FindClassesOfType<IDiscoverable>(AssembliesToScan),
"scanning assemblies",
true);
Expand All @@ -644,7 +681,7 @@ public IEnumerable<Type> GetTypesWithAttribute<T, TAttribute>(bool cache = true,

// filter the cached discovered types (and maybe cache the result)
return GetTypesInternal(
typeof (T), typeof (TAttribute),
typeof(T), typeof(TAttribute),
() => discovered
.Where(x => typeof(T).IsAssignableFrom(x))
.Where(x => x.GetCustomAttributes<TAttribute>(false).Any()),
Expand All @@ -664,7 +701,9 @@ public IEnumerable<Type> GetAttributedTypes<TAttribute>(bool cache = true, IEnum
where TAttribute : Attribute
{
if (_logger == null)
{
throw new InvalidOperationException("Cannot get types from a test/blank type loader.");
}

// do not cache anything from specific assemblies
cache &= specificAssemblies == null;
Expand All @@ -673,7 +712,7 @@ public IEnumerable<Type> GetAttributedTypes<TAttribute>(bool cache = true, IEnum
_logger.Debug<TypeLoader, string>("Running a full, non-cached, scan for types / attribute {AttributeName} (slow).", typeof(TAttribute).FullName);

return GetTypesInternal(
typeof (object), typeof (TAttribute),
typeof(object), typeof(TAttribute),
() => TypeFinder.FindClassesWithAttribute<TAttribute>(specificAssemblies ?? AssembliesToScan),
"scanning assemblies",
cache);
Expand All @@ -693,12 +732,14 @@ private IEnumerable<Type> GetTypesInternal(
var name = GetName(baseType, attributeType);

lock (_locko)
using (_logger.DebugDuration<TypeLoader>(
{
using (_logger.DebugDuration<TypeLoader>(
"Getting " + name,
"Got " + name)) // cannot contain typesFound.Count as it's evaluated before the find
{
// get within a lock & timer
return GetTypesInternalLocked(baseType, attributeType, finder, action, cache);
{
// get within a lock & timer
return GetTypesInternalLocked(baseType, attributeType, finder, action, cache);
}
}
}

Expand All @@ -720,7 +761,9 @@ private IEnumerable<Type> GetTypesInternalLocked(
var listKey = new CompositeTypeTypeKey(baseType ?? tobject, attributeType ?? tobject);
TypeList typeList = null;
if (cache)
{
_types.TryGetValue(listKey, out typeList); // else null
}

// if caching and found, return
if (typeList != null)
Expand Down Expand Up @@ -795,7 +838,9 @@ private IEnumerable<Type> GetTypesInternalLocked(
_logger.Debug<TypeLoader, string>("Getting {TypeName}: " + action + ".", GetName(baseType, attributeType));

foreach (var t in finder())
{
typeList.Add(t);
}
}

// if we are to cache the results, do so
Expand All @@ -807,7 +852,9 @@ private IEnumerable<Type> GetTypesInternalLocked(
_types[listKey] = typeList;
//if we are scanning then update the cache file
if (scan)
{
UpdateCache();
}
}

_logger.Debug<TypeLoader, string, string>("Got {TypeName}, caching ({CacheType}).", GetName(baseType, attributeType), added.ToString().ToLowerInvariant());
Expand Down

0 comments on commit b7c1ad2

Please sign in to comment.