Skip to content

Commit

Permalink
[controls] fix leak in ImageSource, caused by Task that never complet…
Browse files Browse the repository at this point in the history
…es (#21928)

Context: https://github.com/AdamEssenmacher/MemoryToolkit.Maui/tree/main/samples

Testing the above sample, I saw some odd results in a `.gcdump`
related to `UriImageSource` inside a `CollectionView`:

    AsyncTaskMethodBuilder+AsyncStateMachineBox<VoidTaskResult, Microsoft.Maui.Controls.ImageElement+<CancelOldValue>d__10>, Count: 182
        Dictionary+Entry<Int32, Task>[], Count: 182
            Dictionary<Int32, Task> [Static variable Task.s_currentActiveTasks], Count: 1

It appears that we `await` a `Task` that never completes, which is:

https://github.com/dotnet/maui/blob/8e4450cbc14932a6c74aeb8b7bfee9c94eca18b0/src/Controls/src/Core/ImageElement.cs#L129

The `Task` also holds onto the `ImageSource` and whatever memory it
happened to use. That would be ok if the `Task` completed.

I could reproduce the problem in a test:

    [Fact]
    public async Task CancelCompletes()
    {
        var imageSource = new StreamImageSource
        {
            Stream = _ => Task.FromResult<Stream>(new MemoryStream())
        };
        await ((IStreamImageSource)imageSource).GetStreamAsync();
        await imageSource.Cancel(); // This should complete!
    }

This non-completing `Task` can occur when changing `Image.Source`,
which would certainly be bad while scrolling a `CollectionView`!

To fix, I refactored the code slightly and had the problematic case
return:

    return Task.FromResult(false);
  • Loading branch information
jonathanpeppers committed Apr 19, 2024
1 parent 1d635bb commit 4cfdc90
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 6 deletions.
10 changes: 4 additions & 6 deletions src/Controls/src/Core/ImageSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,14 @@ public virtual Task<bool> Cancel()
if (!IsLoading)
return Task.FromResult(false);

var tcs = new TaskCompletionSource<bool>();
TaskCompletionSource<bool> original = Interlocked.CompareExchange(ref _completionSource, tcs, null);
if (original == null)
TaskCompletionSource<bool> original = Interlocked.CompareExchange(ref _completionSource, new TaskCompletionSource<bool>(), null);
if (original is null)
{
_cancellationTokenSource.Cancel();
return Task.FromResult(false);
}
else
tcs = original;

return tcs.Task;
return original.Task;
}

/// <include file="../../docs/Microsoft.Maui.Controls/ImageSource.xml" path="//Member[@MemberName='FromFile']/Docs/*" />
Expand Down
11 changes: 11 additions & 0 deletions src/Controls/tests/Core.UnitTests/ImageSourceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -156,5 +156,16 @@ public void ImplicitCastOnAbsolutePathsShouldCreateAFileImageSource()
var image = new Image { Source = path };
Assert.IsType<FileImageSource>(image.Source);
}

[Fact]
public async Task CancelCompletes()
{
var imageSource = new StreamImageSource
{
Stream = _ => Task.FromResult<Stream>(new MemoryStream())
};
await ((IStreamImageSource)imageSource).GetStreamAsync();
await imageSource.Cancel(); // This should complete!
}
}
}

0 comments on commit 4cfdc90

Please sign in to comment.