Unplanned
Last Updated: 27 Aug 2020 05:54 by ADMIN
Dimitar
Created on: 25 Aug 2020 11:23
Category: RichTextBox
Type: Bug Report
1
RichTextBox: Memory leak caused by the section columns dialog
Memory leak caused by the section columns dialog
2 comments
ADMIN
Dimitar
Posted on: 27 Aug 2020 05:54

Hi Joshua,

Thank you for sharing the details here as well.

Do not hesitate to contact us if you have other questions.

Regards,
Dimitar
Progress Telerik

Virtual Classroom, the free self-paced technical training that gets you up to speed with Telerik and Kendo UI products quickly just got a fresh new look + new and improved content including a brand new Blazor course! Check it out at https://learn.telerik.com/.

Joshua
Posted on: 26 Aug 2020 20:41

This is a two-part bug report that mentions code from this file:
...\Controls\RichTextBox\RichTextBoxUI\Dialogs\SectionColumnsDialog.xaml.cs

Part #1

The WPF constructor for SectionColumnsDialog calls InitializeInputBindings() which in return calls CommandManager.RegisterClassInputBinding() to subscribe an event for the Enter key. RegisterClassInputBinding() is a static method that applies to all instances of the class and it does not use weak references.

        private void InitializeInputBindings()
        {
            KeyBinding keyBinding = new KeyBinding(this.viewModel.ApplyChangesCommand, Key.Enter, ModifierKeys.None);
#if WPF
            CommandManager.RegisterClassInputBinding(this.GetType(), keyBinding);
#elif SILVERLIGHT
            InputBindingCollection inputBindings = new InputBindingCollection();
            inputBindings.Add(keyBinding);
            CommandManager.SetInputBindings(this, inputBindings);
#endif
        }

The problem is that the binding is attached to an instance command property on the viewmodel (SectionColumnsDialogViewModel). This creates a strong root that never gets released (see attached image), and as a result every new SectionColumnsDialog created for a RadRichTextBox stays in memory forever. This could be a soft security risk depending on the contents of the text box. Please use another more local way to capture the Enter key event. This should be standard fare.

Part #2

SectionColumnsDialog additionally subscribes to the viewmodel on two events: ApplyChangesRequested and CancelChangesRequested. When the viewmodel stays in memory forever, the dialog stays in memory forever too, and these two events keep a much larger application object graph in memory. SectionColumnsDialog should use a WeakEventManager to subscribe to these events. XAML views and controls generally subscribe using weak events because they are designed to be garbage collected (notice how none of them have a Dispose method).

Alternatively, SectionColumnsDialog.ShowDialog() could subscribe the two events before displaying the window, and then unsubscribe them in a finally scope after displaying the window, as below:

public void ShowDialog(SectionColumnsDialogContext context)
{
    this.SetOwner(context.Owner);
    this.applySectionColumnsCallback = context.ApplySectionColumnsCallback;

    Section currentSection = context.Owner.GetCurrentSection();

    this.viewModel.InitializeViewModel(currentSection.Columns, currentSection.ActualPageSize.Width, currentSection.ActualPageMargin.Horizontal);

    try
    {
        this.viewModel.ApplyChangesRequested += this.ViewModel_ApplyChangesRequested;
        this.viewModel.CancelChangesRequested += this.ViewModel_CancelChangesRequested;
        this.ShowDialog();
    }
    finally
    {
        this.viewModel.ApplyChangesRequested -= this.ViewModel_ApplyChangesRequested;
        this.viewModel.CancelChangesRequested -= this.ViewModel_CancelChangesRequested;
    }
}


Attached Files: