SixLabors / ImageSharp

:camera: A modern, cross-platform, 2D Graphics library for .NET

https://sixlabors.com/products/imagesharp/

Other

7.26k stars 844 forks source link

Fix async-over-sync issue in Image.DecodeAsync() #2725

Closed kroymann closed 2 months ago

kroymann commented 2 months ago

Prerequisites

[X] I have written a descriptive pull-request title
[X] I have verified that there are no overlapping pull-requests open
[X] I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules :cop:.
[X] I have provided test coverage for my change (where applicable)

Description

While investigating recent instability in our production server environment, we discovered another code path within the async image loading code that would result in a blocking, synchronous stream read, which on our servers was leading to threadpool exhaustion. This change adds a proper asynchronous variant of the offending code so that the stream reading triggered by Image.DecodeAsync() executes asynchronously.

JimBobSquarePants commented 2 months ago

Thanks @kroymann I'll have a proper read over the changes ASAP.

kroymann commented 2 months ago

@antonfirsov Can you tell why the builds are failing after I added the two commits addressing your PR feedback? They seem like innocuous changes, and I can't understand why the builds are now failing when they succeeded before.

antonfirsov commented 2 months ago

---- System.DllNotFoundException : Unable to load shared library 'libgdiplus' or one of its dependencies. In order to help diagnose loading problems, consider setting the DYLD_PRINT_LIBRARIES environment variable: 
dlopen(/Users/runner/work/ImageSharp/ImageSharp/artifacts/bin/tests/ImageSharp.Tests/Release/net8.0/runtimes/osx-arm64/native/libgdiplus.dylib, 0x0001): tried: '/Users/runner/work/ImageSharp/ImageSharp/artifacts/bin/tests/ImageSharp.Tests/Release/net8.0/runtimes/osx-arm64/native/libgdiplus.dylib' (no such file),

@kroymann this doesn't look related to your change. There is probably an issue with the macos-latest image, since restarting the job didn't help.

antonfirsov commented 2 months ago

2728 should fix the mac runs.