Skip to content

Commit

Permalink
Memory Leaks (#14348)
Browse files Browse the repository at this point in the history
  • Loading branch information
jtkech authored Oct 11, 2023
1 parent 919f378 commit 677e5b1
Show file tree
Hide file tree
Showing 49 changed files with 1,909 additions and 380 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@
using Microsoft.AspNetCore.Mvc.ApplicationModels;
using Microsoft.AspNetCore.Mvc.ApplicationParts;
using Microsoft.AspNetCore.Mvc.Razor.Compilation;
using Microsoft.AspNetCore.Mvc.Razor.Extensions;
using Microsoft.Extensions.Hosting;
using OrchardCore.Mvc;

namespace OrchardCore.Admin
{
internal class AdminPageRouteModelProvider : IPageRouteModelProvider
{
private const string RazorPageDocumentKind = "mvc.1.0.razor-page";

private readonly IHostEnvironment _hostingEnvironment;
private readonly ApplicationPartManager _applicationManager;

Expand Down Expand Up @@ -75,7 +74,8 @@ public void OnProvidersExecuted(PageRouteModelProviderContext context)
{
}

private static IEnumerable<CompiledViewDescriptor> GetPageDescriptors<T>(ApplicationPartManager applicationManager) where T : ViewsFeature, new()
private static IEnumerable<CompiledViewDescriptor> GetPageDescriptors<T>(ApplicationPartManager applicationManager)
where T : ViewsFeature, new()
{
if (applicationManager == null)
{
Expand All @@ -100,7 +100,8 @@ public void OnProvidersExecuted(PageRouteModelProviderContext context)
}
}

private static bool IsRazorPage(CompiledViewDescriptor viewDescriptor) => viewDescriptor.Item?.Kind == RazorPageDocumentKind;
private static bool IsRazorPage(CompiledViewDescriptor viewDescriptor) =>
viewDescriptor.Item?.Kind == RazorPageDocumentClassifierPass.RazorPageDocumentKind;

private static T GetViewFeature<T>(ApplicationPartManager applicationManager) where T : ViewsFeature, new()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,10 @@ public async Task InvokeAsync(HttpContext httpContext)
}

// Check if the tenant was installed by another instance.
var settings = await _shellSettingsManager.LoadSettingsAsync(_shellSettings.Name);
using var settings = (await _shellSettingsManager
.LoadSettingsAsync(_shellSettings.Name))
.AsDisposable();

if (!settings.IsUninitialized())
{
await _shellHost.ReloadShellContextAsync(_shellSettings, eventSource: false);
Expand Down Expand Up @@ -204,9 +207,10 @@ public async Task<bool> SetupTenantAsync(ISetupService setupService, TenantSetup
/// <returns>The <see cref="ShellSettings"/>.</returns>
public async Task<ShellSettings> CreateTenantSettingsAsync(TenantSetupOptions setupOptions)
{
var shellSettings = _shellSettingsManager
using var shellSettings = _shellSettingsManager
.CreateDefaultSettings()
.AsUninitialized();
.AsUninitialized()
.AsDisposable();

shellSettings.Name = setupOptions.ShellName;
shellSettings.RequestUrlHost = setupOptions.RequestUrlHost;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,10 @@ public async Task<IActionResult> Create()
var recipes = recipeCollections.SelectMany(x => x).Where(x => x.IsSetupRecipe).OrderBy(r => r.DisplayName).ToArray();

// Creates a default shell settings based on the configuration.
var shellSettings = _shellSettingsManager.CreateDefaultSettings();
using var shellSettings = _shellSettingsManager
.CreateDefaultSettings()
.AsUninitialized()
.AsDisposable();

var currentFeatureProfiles = shellSettings.GetFeatureProfiles();
var featureProfiles = await GetFeatureProfilesAsync(currentFeatureProfiles);
Expand Down Expand Up @@ -348,9 +351,10 @@ public async Task<IActionResult> Create(EditTenantViewModel model)
if (ModelState.IsValid)
{
// Creates a default shell settings based on the configuration.
var shellSettings = _shellSettingsManager
using var shellSettings = _shellSettingsManager
.CreateDefaultSettings()
.AsUninitialized();
.AsUninitialized()
.AsDisposable();

shellSettings.Name = model.Name;
shellSettings.RequestUrlHost = model.RequestUrlHost;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,10 @@ public async Task<IActionResult> Create(TenantApiModel model)
if (model.IsNewTenant)
{
// Creates a default shell settings based on the configuration.
var shellSettings = _shellSettingsManager
using var shellSettings = _shellSettingsManager
.CreateDefaultSettings()
.AsUninitialized();
.AsUninitialized()
.AsDisposable();

shellSettings.Name = model.Name;
shellSettings.RequestUrlHost = model.RequestUrlHost;
Expand All @@ -124,14 +125,15 @@ public async Task<IActionResult> Create(TenantApiModel model)
shellSettings["FeatureProfile"] = string.Join(',', model.FeatureProfiles ?? Array.Empty<string>());

await _shellHost.UpdateShellSettingsAsync(shellSettings);
var reloadedSettings = _shellHost.GetSettings(shellSettings.Name);

var token = CreateSetupToken(shellSettings);
var token = CreateSetupToken(reloadedSettings);

return Ok(GetEncodedUrl(shellSettings, token));
return Ok(GetEncodedUrl(reloadedSettings, token));
}
else
{
// Site already exists, return 201 for indempotency purposes.
// Site already exists, return 201 for idempotency purposes.

var token = CreateSetupToken(settings);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
Expand Down Expand Up @@ -82,7 +81,11 @@ public async Task<IEnumerable<ModelError>> ValidateAsync(TenantModelBase model)
if (existingShellSettings is null)
{
// Set the settings to be validated.
shellSettings = _shellSettingsManager.CreateDefaultSettings();
shellSettings = _shellSettingsManager
.CreateDefaultSettings()
.AsUninitialized()
.AsDisposable();

shellSettings.Name = model.Name;
}
else if (existingShellSettings.IsDefaultShell())
Expand All @@ -106,6 +109,9 @@ public async Task<IEnumerable<ModelError>> ValidateAsync(TenantModelBase model)

if (shellSettings is not null)
{
// A newly loaded settings from the configuration should be disposed.
using var disposable = existingShellSettings is null ? shellSettings : null;

var validationContext = new DbConnectionValidatorContext(shellSettings, model);
await ValidateConnectionAsync(validationContext, errors);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ public async override Task<ActivityExecutionResult> ExecuteAsync(WorkflowExecuti
var featureProfile = (await ExpressionEvaluator.EvaluateAsync(FeatureProfile, workflowContext, null))?.Trim();

// Creates a default shell settings based on the configuration.
var shellSettings = ShellSettingsManager.CreateDefaultSettings();
using var shellSettings = ShellSettingsManager
.CreateDefaultSettings()
.AsUninitialized()
.AsDisposable();

shellSettings.Name = tenantName;

Expand Down Expand Up @@ -172,9 +175,10 @@ public async override Task<ActivityExecutionResult> ExecuteAsync(WorkflowExecuti
shellSettings["Secret"] = Guid.NewGuid().ToString();

await ShellHost.UpdateShellSettingsAsync(shellSettings);
var reloadedSettings = ShellHost.GetSettings(shellSettings.Name);

workflowContext.LastResult = shellSettings;
workflowContext.CorrelationId = shellSettings.Name;
workflowContext.LastResult = reloadedSettings;
workflowContext.CorrelationId = reloadedSettings.Name;

return Outcomes("Done");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,24 @@ namespace OrchardCore.Environment.Shell.Builders
/// </summary>
public class ShellContext : IDisposable, IAsyncDisposable
{
private bool _disposed;
private List<WeakReference<ShellContext>> _dependents;
private readonly SemaphoreSlim _semaphore = new(1);
private bool _disposed;

internal volatile int _refCount;
internal volatile int _terminated;
internal bool _released;

/// <summary>
/// Initializes a new <see cref="ShellContext"/>.
/// </summary>
public ShellContext() => UtcTicks = DateTime.UtcNow.Ticks;

/// <summary>
/// The creation date and time of this shell context in ticks.
/// </summary>
public long UtcTicks { get; }

/// <summary>
/// The <see cref="ShellSettings"/> holding the tenant settings and configuration.
/// </summary>
Expand Down Expand Up @@ -56,7 +66,6 @@ public class PlaceHolder : ShellContext
public PlaceHolder()
{
_released = true;
_disposed = true;
}

/// <summary>
Expand Down Expand Up @@ -101,7 +110,12 @@ public async Task<ShellScope> CreateScopeAsync()
public int ActiveScopes => _refCount;

/// <summary>
/// Mark the <see cref="ShellContext"/> as released and then a candidate to be disposed.
/// Whether or not this instance uses shared <see cref="Settings"/> that should not be disposed.
/// </summary>
public bool SharedSettings { get; internal set; }

/// <summary>
/// Marks the <see cref="ShellContext"/> as released and then a candidate to be disposed.
/// </summary>
public Task ReleaseAsync() => ReleaseInternalAsync();

Expand All @@ -111,6 +125,14 @@ public async Task<ShellScope> CreateScopeAsync()

internal async Task ReleaseInternalAsync(ReleaseMode mode = ReleaseMode.Normal)
{
// A 'PlaceHolder' is always released.
if (this is PlaceHolder)
{
// But still try to dispose the settings.
await DisposeAsync();
return;
}

if (_released)
{
// Prevent infinite loops with circular dependencies
Expand Down Expand Up @@ -239,7 +261,6 @@ private void Close()

_disposed = true;

// Disposes all the services registered for this shell.
if (ServiceProvider is IDisposable disposable)
{
disposable.Dispose();
Expand All @@ -257,7 +278,6 @@ private async ValueTask CloseAsync()

_disposed = true;

// Disposes all the services registered for this shell.
if (ServiceProvider is IAsyncDisposable asyncDisposable)
{
await asyncDisposable.DisposeAsync();
Expand All @@ -276,6 +296,15 @@ private void Terminate()
IsActivated = false;
Blueprint = null;
Pipeline = null;

_semaphore?.Dispose();

if (SharedSettings)
{
return;
}

Settings?.Dispose();
}

~ShellContext() => Close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,23 @@
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.FileProviders;
using Microsoft.Extensions.Primitives;

namespace OrchardCore.Environment.Shell.Configuration.Internal
{
internal class UpdatableDataProvider : IConfigurationProvider, IEnumerable<KeyValuePair<string, string>>
internal class UpdatableDataProvider : IConfigurationProvider, IConfigurationSource, IEnumerable<KeyValuePair<string, string>>
{
private ConfigurationReloadToken _reloadToken = new();
public UpdatableDataProvider() => Data = new(StringComparer.OrdinalIgnoreCase);

public UpdatableDataProvider(IEnumerable<KeyValuePair<string, string>> initialData)
{
Data = new ConcurrentDictionary<string, string>(initialData, StringComparer.OrdinalIgnoreCase);
initialData ??= Enumerable.Empty<KeyValuePair<string, string>>();
Data = new(initialData, StringComparer.OrdinalIgnoreCase);
}

protected IDictionary<string, string> Data { get; set; }

public void Add(string key, string value) => Data.Add(key, value);
protected ConcurrentDictionary<string, string> Data { get; set; }

public IEnumerator<KeyValuePair<string, string>> GetEnumerator() => Data.GetEnumerator();

Expand Down Expand Up @@ -51,17 +50,10 @@ private static string Segment(string key, int prefixLength)
return indexOf < 0 ? key[prefixLength..] : key[prefixLength..indexOf];
}

public IChangeToken GetReloadToken()
{
return _reloadToken;
}

protected void OnReload()
{
var previousToken = Interlocked.Exchange(ref _reloadToken, new ConfigurationReloadToken());
previousToken.OnReload();
}
public IChangeToken GetReloadToken() => NullChangeToken.Singleton;

public override string ToString() => $"{GetType().Name}";

public IConfigurationProvider Build(IConfigurationBuilder builder) => this;
}
}
Loading

0 comments on commit 677e5b1

Please sign in to comment.