diff --git a/src/ImageSharp/Formats/ImageDecoder.cs b/src/ImageSharp/Formats/ImageDecoder.cs index 2ef3c7b75..91d875b69 100644 --- a/src/ImageSharp/Formats/ImageDecoder.cs +++ b/src/ImageSharp/Formats/ImageDecoder.cs @@ -208,13 +208,6 @@ public abstract class ImageDecoder : IImageDecoder stream.Position = position + s.Position; } - // TODO: This is a hack. Our decoders do not check for cancellation requests. - // We need to fix this properly by implemented each decoder. - if (ct.IsCancellationRequested) - { - throw new TaskCanceledException(); - } - return result; } diff --git a/src/ImageSharp/Formats/ImageDecoderUtilities.cs b/src/ImageSharp/Formats/ImageDecoderUtilities.cs index 83cc997af..2a5b91b09 100644 --- a/src/ImageSharp/Formats/ImageDecoderUtilities.cs +++ b/src/ImageSharp/Formats/ImageDecoderUtilities.cs @@ -18,7 +18,7 @@ internal static class ImageDecoderUtilities Stream stream, CancellationToken cancellationToken) { - using BufferedReadStream bufferedReadStream = new(configuration, stream); + using BufferedReadStream bufferedReadStream = new(configuration, stream, cancellationToken); try { @@ -50,7 +50,7 @@ internal static class ImageDecoderUtilities CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - using BufferedReadStream bufferedReadStream = new(configuration, stream); + using BufferedReadStream bufferedReadStream = new(configuration, stream, cancellationToken); try { diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index 291a31886..c5b7270db 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -12,6 +12,8 @@ namespace SixLabors.ImageSharp.IO; /// internal sealed class BufferedReadStream : Stream { + private readonly CancellationToken cancellationToken; + private readonly int maxBufferIndex; private readonly byte[] readBuffer; @@ -33,12 +35,15 @@ internal sealed class BufferedReadStream : Stream /// /// The configuration which allows altering default behaviour or extending the library. /// The input stream. - public BufferedReadStream(Configuration configuration, Stream stream) + /// The optional cancellation token. + public BufferedReadStream(Configuration configuration, Stream stream, CancellationToken cancellationToken = default) { Guard.NotNull(configuration, nameof(configuration)); Guard.IsTrue(stream.CanRead, nameof(stream), "Stream must be readable."); Guard.IsTrue(stream.CanSeek, nameof(stream), "Stream must be seekable."); + this.cancellationToken = cancellationToken; + // Ensure all underlying buffers have been flushed before we attempt to read the stream. // User streams may have opted to throw from Flush if CanWrite is false // (although the abstract Stream does not do so). @@ -163,6 +168,8 @@ internal sealed class BufferedReadStream : Stream [MethodImpl(MethodImplOptions.AggressiveInlining)] public override int Read(Span buffer) { + this.cancellationToken.ThrowIfCancellationRequested(); + // Too big for our buffer. Read directly from the stream. int count = buffer.Length; if (count > this.BufferSize) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.Decode_Cancellation.cs b/tests/ImageSharp.Tests/Image/ImageTests.Decode_Cancellation.cs index 14d65fbb9..7bd794c2c 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.Decode_Cancellation.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.Decode_Cancellation.cs @@ -2,7 +2,7 @@ // Licensed under the Six Labors Split License. using SixLabors.ImageSharp.Formats; -using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestUtilities; namespace SixLabors.ImageSharp.Tests; @@ -13,148 +13,68 @@ public partial class ImageTests private bool isTestStreamSeekable; private readonly SemaphoreSlim notifyWaitPositionReachedSemaphore = new(0); private readonly SemaphoreSlim continueSemaphore = new(0); - private readonly CancellationTokenSource cts = new(); public Decode_Cancellation() => this.TopLevelConfiguration.StreamProcessingBufferSize = 128; - [Theory] - [InlineData(false)] - [InlineData(true)] - public Task LoadAsync_Specific_Stream(bool isInputStreamSeekable) + public static readonly TheoryData TestFiles = new() { - this.isTestStreamSeekable = isInputStreamSeekable; - _ = Task.Factory.StartNew(this.DoCancel, TaskCreationOptions.LongRunning); - - DecoderOptions options = new() - { - Configuration = this.TopLevelConfiguration - }; - - return Assert.ThrowsAsync(() => Image.LoadAsync(options, this.DataStream, this.cts.Token)); - } + TestImages.Png.BikeSmall, + TestImages.Jpeg.Baseline.Jpeg420Small, + TestImages.Bmp.Car, + TestImages.Tiff.RgbUncompressed, + TestImages.Gif.Kumin, + TestImages.Tga.Bit32PalRleBottomLeft, + TestImages.Webp.TestPatternOpaqueSmall, + TestImages.Pbm.RgbPlainMagick + }; [Theory] - [InlineData(false)] - [InlineData(true)] - public Task LoadAsync_Agnostic_Stream(bool isInputStreamSeekable) + [MemberData(nameof(TestFiles))] + public async Task IdentifyAsync_IsCancellable(string file) { - this.isTestStreamSeekable = isInputStreamSeekable; - _ = Task.Factory.StartNew(this.DoCancel, TaskCreationOptions.LongRunning); - - DecoderOptions options = new() + CancellationTokenSource cts = new(); + string path = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, file); + using PausedStream pausedStream = new(path); + pausedStream.OnWaiting(_ => { - Configuration = this.TopLevelConfiguration - }; - - return Assert.ThrowsAsync(() => Image.LoadAsync(options, this.DataStream, this.cts.Token)); - } - - [Fact] - public Task LoadAsync_Agnostic_Path() - { - this.isTestStreamSeekable = true; - _ = Task.Factory.StartNew(this.DoCancel, TaskCreationOptions.LongRunning); + cts.Cancel(); + pausedStream.Release(); + }); + Configuration configuration = Configuration.CreateDefaultInstance(); + configuration.FileSystem = new SingleStreamFileSystem(pausedStream); DecoderOptions options = new() { - Configuration = this.TopLevelConfiguration + Configuration = configuration }; - return Assert.ThrowsAsync(() => Image.LoadAsync(options, this.MockFilePath, this.cts.Token)); - } - - [Fact] - public Task LoadAsync_Specific_Path() - { - this.isTestStreamSeekable = true; - _ = Task.Factory.StartNew(this.DoCancel, TaskCreationOptions.LongRunning); - - DecoderOptions options = new() - { - Configuration = this.TopLevelConfiguration - }; - - return Assert.ThrowsAsync(() => Image.LoadAsync(options, this.MockFilePath, this.cts.Token)); + await Assert.ThrowsAsync(async () => await Image.IdentifyAsync(options, "someFakeFile", cts.Token)); } [Theory] - [InlineData(false)] - [InlineData(true)] - public Task IdentifyAsync_Stream(bool isInputStreamSeekable) - { - this.isTestStreamSeekable = isInputStreamSeekable; - _ = Task.Factory.StartNew(this.DoCancel, TaskCreationOptions.LongRunning); - - DecoderOptions options = new() - { - Configuration = this.TopLevelConfiguration - }; - - return Assert.ThrowsAsync(() => Image.IdentifyAsync(options, this.DataStream, this.cts.Token)); - } - - [Fact] - public Task IdentifyAsync_CustomConfiguration_Path() + [MemberData(nameof(TestFiles))] + public async Task DecodeAsync_IsCancellable(string file) { - this.isTestStreamSeekable = true; - _ = Task.Factory.StartNew(this.DoCancel, TaskCreationOptions.LongRunning); - - DecoderOptions options = new() + CancellationTokenSource cts = new(); + string path = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, file); + using PausedStream pausedStream = new(path); + pausedStream.OnWaiting(_ => { - Configuration = this.TopLevelConfiguration - }; - - return Assert.ThrowsAsync(() => Image.IdentifyAsync(options, this.MockFilePath, this.cts.Token)); - } - - [Theory] - [InlineData(false)] - [InlineData(true)] - public Task IdentifyWithFormatAsync_CustomConfiguration_Stream(bool isInputStreamSeekable) - { - this.isTestStreamSeekable = isInputStreamSeekable; - _ = Task.Factory.StartNew(this.DoCancel, TaskCreationOptions.LongRunning); + cts.Cancel(); + pausedStream.Release(); + }); + Configuration configuration = Configuration.CreateDefaultInstance(); + configuration.FileSystem = new SingleStreamFileSystem(pausedStream); DecoderOptions options = new() { - Configuration = this.TopLevelConfiguration + Configuration = configuration }; - return Assert.ThrowsAsync(() => Image.IdentifyWithFormatAsync(options, this.DataStream, this.cts.Token)); - } - - [Fact] - public Task IdentifyWithFormatAsync_CustomConfiguration_Path() - { - this.isTestStreamSeekable = true; - _ = Task.Factory.StartNew(this.DoCancel, TaskCreationOptions.LongRunning); - - DecoderOptions options = new() + await Assert.ThrowsAsync(async () => { - Configuration = this.TopLevelConfiguration - }; - - return Assert.ThrowsAsync(() => Image.IdentifyWithFormatAsync(options, this.MockFilePath, this.cts.Token)); - } - - [Fact] - public Task IdentifyWithFormatAsync_DefaultConfiguration_Stream() - { - _ = Task.Factory.StartNew(this.DoCancel, TaskCreationOptions.LongRunning); - - return Assert.ThrowsAsync(() => Image.IdentifyWithFormatAsync(this.DataStream, this.cts.Token)); - } - - private async Task DoCancel() - { - // wait until we reach the middle of the steam - await this.notifyWaitPositionReachedSemaphore.WaitAsync(); - - // set the cancellation - this.cts.Cancel(); - - // continue processing the stream - this.continueSemaphore.Release(); + using Image image = await Image.LoadAsync(options, "someFakeFile", cts.Token); + }); } protected override Stream CreateStream() => this.TestFormat.CreateAsyncSemaphoreStream(this.notifyWaitPositionReachedSemaphore, this.continueSemaphore, this.isTestStreamSeekable);