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

[AndyButland]

Prerequisites

Addresses #20709

Description

This is the 17 version of #20722 (that I'll also backport to 16 once approved).

As per the other PR, the fixes are either adopting the disposable pattern, or where registered transient, removing the OnChange handler as it's unnecessary if the class isn't long lived.

[Copilot]

Pull Request Overview

This PR refactors configuration settings handling across multiple classes by removing dynamic OnChange subscriptions and making settings immutable (readonly). The primary goal is to prevent memory leaks from subscriptions that were never properly disposed.

• Converted mutable settings fields to readonly fields, capturing configuration values only at construction time
• Removed OnChange subscription handlers that were updating settings dynamically
• Added IDisposable implementation to classes that still need subscriptions (value editors and converters)
• Made fields readonly in classes where subscriptions were completely removed

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
EmailUserForgotPasswordSender.cs	Made GlobalSettings and SecuritySettings fields readonly, removed OnChange subscriptions
IndexingNotificationHandler.DeliveryApi.cs	Made DeliveryApiSettings field readonly, removed OnChange subscription
RteBlockRenderingValueConverter.cs	Made DeliveryApiSettings readonly, stored subscription as IDisposable field, implemented IDisposable
ImageCropperPropertyValueEditor.cs	Made ContentSettings readonly, stored subscription as IDisposable field, implemented IDisposable
ImageCropperPropertyEditor.cs	Made ContentSettings readonly, stored subscription as IDisposable field, implemented IDisposable, removed null checks
FileUploadPropertyValueEditor.cs	Made ContentSettings readonly, stored subscription as IDisposable field, implemented IDisposable, removed null checks
ApiPublishedContentCache.cs	Removed unused imports
ModelsBuilderPresentationFactory.cs	Made ModelsBuilderSettings field readonly, removed OnChange subscription, cleaned up imports
BuildModelsBuilderController.cs	Made ModelsBuilderSettings field readonly, removed OnChange subscription, cleaned up imports
GetHelpController.cs	Made HelpPageSettings field readonly, removed OnChange subscription and helper method

[Migaroez]

Had a look over the codebase and can't find any other cases where we can make meaningfull improvments as all other event subscribers are either

• subclasses of the event publisher: same lifetime
• the subscribers are DI singletons: they have application lifetime so they cant have a shorter lifetime than the publisher
• the subscribers are registered into a builderscollection which shouldn't be mutated after the compute phase which makes them equivalent to the previous scenario.

The only thing i would change is to remove the changes to the propertyEditor so the intent of the PR is to only implement changes to things where we see risks in normal operation/implementation.

[AndyButland]

The only thing i would change is to remove the changes to the propertyEditor so the intent of the PR is to only implement changes to things where we see risks in normal operation/implementation.

Could you be a bit more specific about what you are suggesting to revert here please?