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

Fix memory leak with IOptionsMonitor.OnChange and non-singleton registered components (closes #20709 for 16/17) (#20723)

Browse Source 
* Fix memory leak with IOptionsMonitor.OnChange and non-singleton registered components.

* Dispose disposable data editors in ValueEditorCache.

* Removed unnecessary refactoring and clarified code comments.
# Conflicts:
#	src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyEditor.cs
This commit is contained in:
Andy Butland
2025-11-06 17:22:43 +01:00
parent e3c394b109
commit ea142d51b7
11 changed files with 65 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/Umbraco.Cms.Api.Management/Controllers/Help/GetHelpController.cs
View File

@@ -17,7 +17,7 @@ public class GetHelpController : HelpControllerBase
{
private readonly ILogger<GetHelpController> _logger;
private readonly IJsonSerializer _jsonSerializer;
private HelpPageSettings _helpPageSettings;
private readonly HelpPageSettings _helpPageSettings;
public GetHelpController(
IOptionsMonitor<HelpPageSettings> helpPageSettings,
@@ -27,11 +27,8 @@ public class GetHelpController : HelpControllerBase
_logger = logger;
_jsonSerializer = jsonSerializer;
_helpPageSettings = helpPageSettings.CurrentValue;
helpPageSettings.OnChange(UpdateHelpPageSettings);
}
private void UpdateHelpPageSettings(HelpPageSettings settings) => _helpPageSettings = settings;
[HttpGet]
[MapToApiVersion("1.0")]
[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)]

8
src/Umbraco.Cms.Api.Management/Controllers/ModelsBuilder/BuildModelsBuilderController.cs
View File

@@ -1,22 +1,18 @@
using System;
using System.Threading.Tasks;
using Asp.Versioning;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Infrastructure.ModelsBuilder;
using Umbraco.Cms.Infrastructure.ModelsBuilder.Building;
using Umbraco.Extensions;
namespace Umbraco.Cms.Api.Management.Controllers.ModelsBuilder;
[ApiVersion("1.0")]
public class BuildModelsBuilderController : ModelsBuilderControllerBase
{
private ModelsBuilderSettings _modelsBuilderSettings;
private readonly ModelsBuilderSettings _modelsBuilderSettings;
private readonly ModelsGenerationError _mbErrors;
private readonly IModelsGenerator _modelGenerator;
@@ -28,8 +24,6 @@ public class BuildModelsBuilderController : ModelsBuilderControllerBase
_mbErrors = mbErrors;
_modelGenerator = modelGenerator;
_modelsBuilderSettings = modelsBuilderSettings.CurrentValue;
modelsBuilderSettings.OnChange(x => _modelsBuilderSettings = x);
}
[HttpPost("build")]

10
src/Umbraco.Cms.Api.Management/Factories/ModelsBuilderPresentationFactory.cs
View File

@@ -1,30 +1,24 @@
using System.Text;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Api.Management.ViewModels.ModelsBuilderDashboard;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Configuration;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Infrastructure.ModelsBuilder;
using Umbraco.Cms.Api.Management.ViewModels.ModelsBuilderDashboard;
using Umbraco.Extensions;
namespace Umbraco.Cms.Api.Management.Factories;
public class ModelsBuilderPresentationFactory : IModelsBuilderPresentationFactory
{
private ModelsBuilderSettings _modelsBuilderSettings;
private readonly ModelsGenerationError _mbErrors;
private readonly OutOfDateModelsStatus _outOfDateModels;
private readonly ModelsBuilderSettings _modelsBuilderSettings;
public ModelsBuilderPresentationFactory(IOptionsMonitor<ModelsBuilderSettings> modelsBuilderSettings, ModelsGenerationError mbErrors, OutOfDateModelsStatus outOfDateModels)
{
_mbErrors = mbErrors;
_outOfDateModels = outOfDateModels;
_modelsBuilderSettings = modelsBuilderSettings.CurrentValue;
modelsBuilderSettings.OnChange(x => _modelsBuilderSettings = x);
}
public ModelsBuilderResponseModel Create() =>
new()
{

8
src/Umbraco.Core/Cache/ValueEditorCache.cs
View File

@@ -51,9 +51,17 @@ public class ValueEditorCache : IValueEditorCache
{
foreach (Dictionary<int, IDataValueEditor> editors in _valueEditorCache.Values)
{
if (editors.TryGetValue(id, out IDataValueEditor? editor))
{
if (editor is IDisposable disposable)
{
disposable.Dispose();
}
editors.Remove(id);
}
}
}
}
}
}

3
src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs
View File

@@ -1,10 +1,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.PublishedCache;
using Umbraco.Cms.Core.Services;
using Umbraco.Extensions;
namespace Umbraco.Cms.Core.DeliveryApi;

19
src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs
View File

@@ -2,6 +2,7 @@
// See LICENSE for more details.
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
@@ -22,14 +23,22 @@ namespace Umbraco.Cms.Core.PropertyEditors;
/// <summary>
/// The value editor for the file upload property editor.
/// </summary>
internal sealed class FileUploadPropertyValueEditor : DataValueEditor
/// <remarks>
/// As this class is loaded into <see cref="ValueEditorCache"/> which can be cleared, it needs
/// to be disposable in order to properly clean up resources such as
/// the settings change subscription and avoid a memory leak.
/// </remarks>
internal sealed class FileUploadPropertyValueEditor : DataValueEditor, IDisposable
{
private readonly MediaFileManager _mediaFileManager;
private readonly ITemporaryFileService _temporaryFileService;
private readonly IScopeProvider _scopeProvider;
private readonly IFileStreamSecurityValidator _fileStreamSecurityValidator;
private readonly FileUploadValueParser _valueParser;
private ContentSettings _contentSettings;
private readonly IDisposable? _contentSettingsChangeSubscription;
/// <summary>
/// Initializes a new instance of the <see cref="FileUploadPropertyValueEditor"/> class.
@@ -46,14 +55,14 @@ internal sealed class FileUploadPropertyValueEditor : DataValueEditor
IFileStreamSecurityValidator fileStreamSecurityValidator)
: base(shortStringHelper, jsonSerializer, ioHelper, attribute)
{
_mediaFileManager = mediaFileManager ?? throw new ArgumentNullException(nameof(mediaFileManager));
_mediaFileManager = mediaFileManager;
_temporaryFileService = temporaryFileService;
_scopeProvider = scopeProvider;
_fileStreamSecurityValidator = fileStreamSecurityValidator;
_valueParser = new FileUploadValueParser(jsonSerializer);
_contentSettings = contentSettings.CurrentValue ?? throw new ArgumentNullException(nameof(contentSettings));
contentSettings.OnChange(x => _contentSettings = x);
_contentSettings = contentSettings.CurrentValue;
_contentSettingsChangeSubscription = contentSettings.OnChange(x => _contentSettings = x);
Validators.Add(new TemporaryFileUploadValidator(
() => _contentSettings,
@@ -216,4 +225,6 @@ internal sealed class FileUploadPropertyValueEditor : DataValueEditor
// in case we are using the old path scheme, try to re-use numbers (bah...)
return _mediaFileManager.GetMediaPath(file.FileName, contentKey, propertyTypeKey);
}
public void Dispose() => _contentSettingsChangeSubscription?.Dispose();
}

21
src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyEditor.cs
View File

@@ -23,9 +23,12 @@ namespace Umbraco.Cms.Core.PropertyEditors;
Constants.PropertyEditors.Aliases.ImageCropper,
ValueType = ValueTypes.Json,
ValueEditorIsReusable = true)]
public class ImageCropperPropertyEditor : DataEditor, IMediaUrlGenerator,
INotificationHandler<ContentCopiedNotification>, INotificationHandler<ContentDeletedNotification>,
INotificationHandler<MediaDeletedNotification>, INotificationHandler<MediaSavingNotification>,
public class ImageCropperPropertyEditor : DataEditor,
IMediaUrlGenerator,
INotificationHandler<ContentCopiedNotification>,
INotificationHandler<ContentDeletedNotification>,
INotificationHandler<MediaDeletedNotification>,
INotificationHandler<MediaSavingNotification>,
INotificationHandler<MemberDeletedNotification>
{
private readonly UploadAutoFillProperties _autoFillProperties;
@@ -33,9 +36,10 @@ public class ImageCropperPropertyEditor : DataEditor, IMediaUrlGenerator,
private readonly IIOHelper _ioHelper;
private readonly ILogger<ImageCropperPropertyEditor> _logger;
private readonly MediaFileManager _mediaFileManager;
private ContentSettings _contentSettings;
private readonly IJsonSerializer _jsonSerializer;
private ContentSettings _contentSettings;
/// <summary>
/// Initializes a new instance of the <see cref="ImageCropperPropertyEditor" /> class.
/// </summary>
@@ -50,15 +54,14 @@ public class ImageCropperPropertyEditor : DataEditor, IMediaUrlGenerator,
IJsonSerializer jsonSerializer)
: base(dataValueEditorFactory)
{
_mediaFileManager = mediaFileManager ?? throw new ArgumentNullException(nameof(mediaFileManager));
_contentSettings = contentSettings.CurrentValue ?? throw new ArgumentNullException(nameof(contentSettings));
_ioHelper = ioHelper ?? throw new ArgumentNullException(nameof(ioHelper));
_autoFillProperties =
uploadAutoFillProperties ?? throw new ArgumentNullException(nameof(uploadAutoFillProperties));
_mediaFileManager = mediaFileManager;
_ioHelper = ioHelper;
_autoFillProperties = uploadAutoFillProperties;
_contentService = contentService;
_jsonSerializer = jsonSerializer;
_logger = loggerFactory.CreateLogger<ImageCropperPropertyEditor>();
_contentSettings = contentSettings.CurrentValue;
contentSettings.OnChange(x => _contentSettings = x);
SupportsReadOnly = true;
}

18
src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs
View File

@@ -22,17 +22,24 @@ namespace Umbraco.Cms.Core.PropertyEditors;
/// <summary>
/// The value editor for the image cropper property editor.
/// </summary>
internal sealed class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core vs web?
/// <remarks>
/// As this class is loaded into <see cref="ValueEditorCache"/> which can be cleared, it needs
/// to be disposable in order to properly clean up resources such as
/// the settings change subscription and avoid a memory leak.
/// </remarks>
internal sealed class ImageCropperPropertyValueEditor : DataValueEditor, IDisposable
{
private readonly IDataTypeConfigurationCache _dataTypeConfigurationCache;
private readonly IFileStreamSecurityValidator _fileStreamSecurityValidator;
private readonly ILogger<ImageCropperPropertyValueEditor> _logger;
private readonly MediaFileManager _mediaFileManager;
private readonly IJsonSerializer _jsonSerializer;
private ContentSettings _contentSettings;
private readonly ITemporaryFileService _temporaryFileService;
private readonly IScopeProvider _scopeProvider;
private ContentSettings _contentSettings;
private readonly IDisposable? _contentSettingsChangeSubscription;
public ImageCropperPropertyValueEditor(
DataEditorAttribute attribute,
ILogger<ImageCropperPropertyValueEditor> logger,
@@ -50,12 +57,13 @@ internal sealed class ImageCropperPropertyValueEditor : DataValueEditor // TODO:
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
_mediaFileManager = mediaFileSystem ?? throw new ArgumentNullException(nameof(mediaFileSystem));
_jsonSerializer = jsonSerializer;
_contentSettings = contentSettings.CurrentValue;
_temporaryFileService = temporaryFileService;
_scopeProvider = scopeProvider;
_fileStreamSecurityValidator = fileStreamSecurityValidator;
_dataTypeConfigurationCache = dataTypeConfigurationCache;
contentSettings.OnChange(x => _contentSettings = x);
_contentSettings = contentSettings.CurrentValue;
_contentSettingsChangeSubscription = contentSettings.OnChange(x => _contentSettings = x);
Validators.Add(new TemporaryFileUploadValidator(() => _contentSettings, TryParseTemporaryFileKey, TryGetTemporaryFile));
}
@@ -267,4 +275,6 @@ internal sealed class ImageCropperPropertyValueEditor : DataValueEditor // TODO:
return filepath;
}
public void Dispose() => _contentSettingsChangeSubscription?.Dispose();
}

9
src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteBlockRenderingValueConverter.cs
View File

@@ -26,7 +26,7 @@ namespace Umbraco.Cms.Core.PropertyEditors.ValueConverters;
/// used dynamically.
/// </summary>
[DefaultPropertyValueConverter]
public class RteBlockRenderingValueConverter : SimpleRichTextValueConverter, IDeliveryApiPropertyValueConverter
public class RteBlockRenderingValueConverter : SimpleRichTextValueConverter, IDeliveryApiPropertyValueConverter, IDisposable
{
private readonly HtmlImageSourceParser _imageSourceParser;
private readonly HtmlLocalLinkParser _linkParser;
@@ -41,7 +41,9 @@ public class RteBlockRenderingValueConverter : SimpleRichTextValueConverter, IDe
private readonly RichTextBlockPropertyValueConstructorCache _constructorCache;
private readonly IVariationContextAccessor _variationContextAccessor;
private readonly BlockEditorVarianceHandler _blockEditorVarianceHandler;
private DeliveryApiSettings _deliveryApiSettings;
private readonly IDisposable? _deliveryApiSettingsChangeSubscription;
public RteBlockRenderingValueConverter(HtmlLocalLinkParser linkParser, HtmlUrlParser urlParser, HtmlImageSourceParser imageSourceParser,
IApiRichTextElementParser apiRichTextElementParser, IApiRichTextMarkupParser apiRichTextMarkupParser,
@@ -62,8 +64,9 @@ public class RteBlockRenderingValueConverter : SimpleRichTextValueConverter, IDe
_logger = logger;
_variationContextAccessor = variationContextAccessor;
_blockEditorVarianceHandler = blockEditorVarianceHandler;
_deliveryApiSettings = deliveryApiSettingsMonitor.CurrentValue;
deliveryApiSettingsMonitor.OnChange(settings => _deliveryApiSettings = settings);
_deliveryApiSettingsChangeSubscription = deliveryApiSettingsMonitor.OnChange(settings => _deliveryApiSettings = settings);
}
public override PropertyCacheLevel GetPropertyCacheLevel(IPublishedPropertyType propertyType) =>
@@ -250,4 +253,6 @@ public class RteBlockRenderingValueConverter : SimpleRichTextValueConverter, IDe
public required RichTextBlockModel? RichTextBlockModel { get; set; }
}
public void Dispose() => _deliveryApiSettingsChangeSubscription?.Dispose();
}

5
src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.DeliveryApi.cs
View File

@@ -1,4 +1,4 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
@@ -17,7 +17,7 @@ internal sealed class DeliveryApiContentIndexingNotificationHandler :
{
private readonly IDeliveryApiIndexingHandler _deliveryApiIndexingHandler;
private readonly ILogger<DeliveryApiContentIndexingNotificationHandler> _logger;
private DeliveryApiSettings _deliveryApiSettings;
private readonly DeliveryApiSettings _deliveryApiSettings;
public DeliveryApiContentIndexingNotificationHandler(
IDeliveryApiIndexingHandler deliveryApiIndexingHandler,
@@ -27,7 +27,6 @@ internal sealed class DeliveryApiContentIndexingNotificationHandler :
_deliveryApiIndexingHandler = deliveryApiIndexingHandler;
_logger = logger;
_deliveryApiSettings = deliveryApiSettings.CurrentValue;
deliveryApiSettings.OnChange(settings => _deliveryApiSettings = settings);
}
public void Handle(ContentCacheRefresherNotification notification)

7
src/Umbraco.Infrastructure/Security/EmailUserForgotPasswordSender.cs
View File

@@ -16,8 +16,8 @@ public class EmailUserForgotPasswordSender : IUserForgotPasswordSender
{
private readonly IEmailSender _emailSender;
private readonly ILocalizedTextService _localizedTextService;
private GlobalSettings _globalSettings;
private SecuritySettings _securitySettings;
private readonly GlobalSettings _globalSettings;
private readonly SecuritySettings _securitySettings;
public EmailUserForgotPasswordSender(
IEmailSender emailSender,
@@ -29,9 +29,6 @@ public class EmailUserForgotPasswordSender : IUserForgotPasswordSender
_localizedTextService = localizedTextService;
_globalSettings = globalSettings.CurrentValue;
_securitySettings = securitySettings.CurrentValue;
globalSettings.OnChange(settings => _globalSettings = settings);
securitySettings.OnChange(settings => _securitySettings = settings);
}
public async Task SendForgotPassword(UserForgotPasswordMessage messageModel)