Unstable and floating exception with the access of the file after reading it in Image.NewFromFile

[AndrePScope]

Hi @kleisauke!

I got an exception and think that it relates to the implementation of NetVips (or LibVips)
System.IO.IOException: The process cannot access the file 'test-X.png' because it is being used by another process

Explanation

So, we got an unstable and floating exception with the access of the file that was created via System.Diagnostics.Process in async operation and which was read in Image.NewFromFile after that.

So for the reproducing, needs 3 things:

1. creates files in the async method via System.Diagnostics.Process in some temp directory
2. read files via Image.NewFromFile
3. try to delete the temp directory

The exception occurs on deleting the temp directory on the 3 step.

Why I think the issue relates to the NetVips (libvips).

1. The first reason, if I do not read files via Image.NewFromFile (2 step), then deletion temp directory works stable and well.

2. The second reason...
   If I do:
   Image.NewFromBuffer(File.ReadAllBytes(filePath), access: Enums.Access.Sequential)
   instead of
   Image.NewFromFile(filePath, access: Enums.Access.Sequential)
   the code works stable and well also (no exception).

Resources for reproducing

Bellow, I listed the full test code where you can reproduce the issue.
Also, here the project with test code for easy reproducing: TestNetVipsAsync.zip
This is the test picture (test.png) needed for this test:

Important!
For reproducing this issue, the test picture must be quite small (as I created).

Full test example

(Please, create the directory C:\Temp\TestNetVips and put there the picture test.png)

static async Task Main(string[] args)
{
	Console.WriteLine("Hello NetVips!");

	string basePath = "C:\\Temp\\TestNetVips\\";
	Directory.SetCurrentDirectory(basePath);

	var testPngFile = "test.png";
	var testDirectory = "TestDirectory";
	Directory.CreateDirectory(testDirectory);

	// Prepare 50 test files (clone 50 from test.png)
	for (int i = 0; i < 50; i++)
		File.Copy(testPngFile, Path.Combine(testDirectory, $"test-{i}.png"), true);

	async Task DoAll(string tempDirectory)
	{
		try
		{
			// prepare directory for converting
			Directory.CreateDirectory(tempDirectory);

			// Create test files for converting via external process
			// System.Diagnostics.Process
			using (var proc = Process.Start(new ProcessStartInfo
			{
				FileName = "xcopy",
				Arguments = $"/S /E /Q /Y {basePath}\\{testDirectory}\\*.* {Path.Combine(basePath, tempDirectory) }\\*.*\r\n",
				WorkingDirectory = "C:\\Temp",
				UseShellExecute = false,
				CreateNoWindow = true
			}))
			{
				proc?.WaitForExit();
			}

			// Do any await operation to make method really async and run it in different thread
			await File.ReadAllBytesAsync("test.png");
			// Ensure we are in different threads
			Debug.WriteLine($"Work with {tempDirectory} in {Thread.CurrentThread.ManagedThreadId} thread");

			// Open files for converting
			foreach (var filePath in Directory.GetFiles(tempDirectory, "test-*.png", SearchOption.TopDirectoryOnly))
			{
				using (Image image = Image.NewFromBuffer(File.ReadAllBytes(filePath), access: Enums.Access.Sequential))
				{
					// Do nothing, just read, this is already enough for issue
					//image.Jpegsave(".....", 75);
				}
			}

			// Delete temp directory !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
			// Here sometimes unexpectedly we got (this is an issue):
			// "System.IO.IOException: The process cannot access the file 'test-X.png' because it is being used by another process"
			Directory.Delete(tempDirectory, true);
		}
		catch (Exception e)
		{
			Debug.WriteLine(e);
			throw;
		}
	}


	try
	{
		// 10 iteration because the exception not stable
		for (int i = 0; i < 10; i++)
		{
			// 20 parallel tasks for reading files via NetVips
			var tasks = new List<Task>();
			for (int j = 0; j < 20; j++)
				tasks.Add(DoAll($"TempDirectory-{i}-{j}"));

			await Task.WhenAll(tasks);
		}
	}
	catch (Exception e)
	{
		Console.WriteLine(e);
		throw;
	}
}

[kleisauke]

Hello, I think the file is being held open in libvips' cache, given that this only occurs with Image.NewFromFile.

Please see #6 (comment) and the corresponding libvips documentation for a way to disable this. AFAIK, this problem only occurs on Windows, other platforms let you delete open files.

[AndrePScope]

Hi @kleisauke !

Thank you for your quickly answer.
But I have to say that reason of issue not in Cahce.

I did this:

NetVips.ModuleInitializer.Initialize();
NetVips.Cache.Max = 0;
NetVips.Cache.MaxFiles = 0;

But problem still here:

The test code works really with many exemplars of one file, but in the real environment the files all different. And exception also there...

Please, test my code...

[kleisauke]

I was able to reproduce it, and after some testing, I was able to resolve it with this patch:

--- a/issue-139.cs
+++ b/issue-139.cs
@@ -2,6 +2,8 @@ static async Task Main(string[] args)
 {
 	Console.WriteLine("Hello NetVips!");
 
+	Cache.MaxFiles = 0;
+
 	string basePath = "C:\\Temp\\TestNetVips\\";
 	Directory.SetCurrentDirectory(basePath);
 
@@ -31,7 +31,7 @@ static async Task Main(string[] args)
 				CreateNoWindow = true
 			}))
 			{
-				proc?.WaitForExit();
+				await proc?.WaitForExitAsync();
 			}
 
 			// Do any await operation to make method really async and run it in different thread

(tested 20 times without problems)

So, it looks like a race condition between the xcopy command and whenever libvips opens a file. This comment might also be relevant here:
https://github.com/dotnet/runtime/blob/4ac2aaa87db417e57715abe63078bb6a2d8a18a4/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs#L1424-L1458

There might be still a change you'll get a System.UnauthorizedAccessException exception on Windows due to Thumbs.db, search indexers and/or anti-virus software. In this case, it might be better to delete the image immediately after you have used it. For example:

--- a/issue-139.cs
+++ b/issue-139.cs
@@ -47,12 +49,13 @@ static async Task Main(string[] args)
 					// Do nothing, just read, this is already enough for issue
 					//image.Jpegsave(".....", 75);
 				}
+				File.Delete(filePath);
 			}
 
 			// Delete temp directory !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 			// Here sometimes unexpectedly we got (this is an issue):
 			// "System.IO.IOException: The process cannot access the file 'test-X.png' because it is being used by another process"
-			Directory.Delete(tempDirectory, true);
+			Directory.Delete(tempDirectory, false);
 		}
 		catch (Exception e)
 		{

[AndrePScope]

Hi @kleisauke !

Thank you very much for your research and for your conclusion!

Really, I did not test WaitForExitAsync because our project still developing in .Net Core 3.1, and WaitForExitAsync method presents only in .Net Core 5.0

As for now, we cannot change the framework in our project.
(btw we have our own implementation of WaitForExitAsync, but it works with the same issue as WaitForExit)

So, I see that the problem in WaitForExit of .Net Core 3.1, and the correct way is to move the project to .Net Core 5.0.

Then I would like to ask your advice...
While our project is running on 3.1, is the correct solution will be do:
Image.NewFromBuffer(File.ReadAllBytes(filePath), access: Enums.Access.Sequential)
instead of
Image.NewFromFile(filePath, access: Enums.Access.Sequential)
or you can suggest something else?

(NewFromBuffer and File.ReadAllBytes(filePath) resolve the issue in .Net Core 3.1 but I am not sure that it is the best...)

Thank you very much again, and I will wait for your advice.

[kleisauke]

Ah, I didn't realize that WaitForExitAsync is only for .NET Core 5.0+. If you want, I can retest this with .NET Core 3.1, and see if there is way to fix this with this requirement.

The Image.NewFromFile -> Image.NewFromBuffer change is more like a workaround than a solution. I think you could also workaround this with the Image.NewFromStream function to avoid loading the entire file into memory, for example:

net-vips/tests/NetVips.Tests/ForeignTests.cs

Lines 1055 to 1060 in 087f9f7

	// buffer-based loader fallback
	using (var input = File.OpenRead(Helper.GifFile))
	{
	var img = Image.NewFromStream(input, access: Enums.Access.Sequential);
	GifValid(img);
	}

[AndrePScope]

Hi @kleisauke !

Thank you for your help.
Yes, if you may, please test the code under 3.1

As for NewFromStream, I think we cannot use this because here we have another problem and another exception.

The deal is that we have some approach (with some methods/wrappers) that allows writing nice and readable code which deletes objects immediately when it becomes unnecessary. In this case, the main code is not overloaded with Dispose operations or using variables.
It works well for NewFromFile, but not for NewFromStream method. When we use NewFromStream we got exceptions.
These exceptions are also floatable...

Sometimes we got this one:
Process terminated. A callback was made on a garbage collected delegate of type 'NetVips!NetVips.Internal.VipsSourceCustom+ReadSignal::Invoke'.

Sometimes another one:
The program '[31364] TestNetVipsAsync.exe' has exited with code -1073741819 (0xc0000005) 'Access violation'.

I changed the previous example and illustrated the problem here: TestNetVipsAsync-2.zip

Here is the main part of the example code.
Pay attention, if we delete the intermediate Image-object before Saving it leads (sometimes, not always) to a problem.

if (Directory.Exists(resultDirectory))
	Directory.Delete(resultDirectory, true);
Directory.CreateDirectory(resultDirectory);

// Open files for converting
foreach (var filePath in Directory.GetFiles(tempDirectory, "test-*.png", SearchOption.TopDirectoryOnly))
{
	using (var input = File.OpenRead(filePath))
	{
		var image = Image.NewFromStream(input, access: Enums.Access.Sequential);

		// Do some transformation, f.e. add alpha
		using var imageWithAlpha = image.Bandjoin(255);

		// !!!!! ------ Dispose in this place make the code wrong and occur exception
		// ==========================================================================
		image.Dispose();

		// !!!!! Do save!
		imageWithAlpha.Jpegsave(Path.Combine(resultDirectory, Path.GetFileNameWithoutExtension(filePath) + ".jpg"), 75);

		// !!!!! +++++ Dispose in this place make the code correct
		// ==========================================================================
		//image.Dispose();
	}
}

Thank you in advance!

[kleisauke]

I retested this with .NET Core 3.1, and was able to resolve it with this compatibility extension method for WaitForExitAsync:

internal static class ExtensionMethods
{
    /// <summary>
    /// Waits asynchronously for the process to exit.
    /// </summary>
    /// <param name="process">The process to wait for cancellation.</param>
    /// <param name="cancellationToken">A cancellation token. If invoked, the task will return 
    /// immediately as canceled.</param>
    /// <returns>A Task representing waiting for the process to end.</returns>
    public static Task WaitForExitAsync(this Process process,
        CancellationToken cancellationToken = default)
    {
        if (process.HasExited) return Task.CompletedTask;

        var tcs = new TaskCompletionSource<object>();
        process.EnableRaisingEvents = true;
        process.Exited += (sender, args) => tcs.TrySetResult(null);
        if (cancellationToken != default)
            cancellationToken.Register(() => tcs.SetCanceled());

        return process.HasExited ? Task.CompletedTask : tcs.Task;
    }
}

(from https://stackoverflow.com/a/19104345)

I also could reproduce the issue with the Image.NewFromStream method. Commit 8682c66 should fix this. See comment #141 (comment) for instructions on how to test that.

[AndrePScope]

Hi @kleisauke !

Thank you very much for your research and help!
Sorry, I could not retest your solution and answer in time, because was sick.

Unfortunately, I have to say, that problem did not resolve still...

Yes, I also had no error in the initial code with your fix.
So, I included this fix in the real project and realized that problem was there still.

Then I moved back to this test tool and just increased the count of parallel tasks from 20 to 30... and... got the same error.

You can also try to test this case.

Moreover, I saw that this error occurs in the standard WaitForExitAsync method in .Net 5.0 (with the case of 30 parallel tasks)!

So, as I see we still have to use

 Image.NewFromBuffer(File.ReadAllBytes(filePath)... )

In this case, the code works well and is stable.

@kleisauke, we truly thank you for your research and wasted time.
If you will have any ideas for solving the problem, we will be sincerely grateful.

Thank you!

[kleisauke]

No problem. I was able to reproduce this issue with the v8.11.4 binaries released on NuGet. However, I couldn't reproduce it with the unreleased v8.12.0 binaries built from the master branch of libvips. Presumably this issue is the same as the one mentioned in comment #135 (comment), but setting NetVips.Concurrency = 1; doesn't seems to help here.

The pre-compiled libvips Windows binaries build from the master branch can be downloaded here (for testing purposes):
https://libvips-packaging.s3.amazonaws.com/vips-dev-w64-web-8.12.0-b2527da-static.zip

(This was build from commit libvips/build-win64-mxe@b09c142)

[kleisauke]

FWIW, I'm testing with these changes:

Changeset

--- a/issue-139.cs
+++ b/issue-139.cs
@@ -2,6 +2,8 @@
 {
 	Console.WriteLine("Hello NetVips!");
 
+	Cache.Max = 0;
+
 	string basePath = "C:\\Temp\\TestNetVips\\";
 	Directory.SetCurrentDirectory(basePath);
 
@@ -31,7 +33,7 @@
 				CreateNoWindow = true
 			}))
 			{
-				proc?.WaitForExit();
+				await proc?.WaitForExitAsync();
 			}
 
 			// Do any await operation to make method really async and run it in different thread
@@ -42,7 +44,7 @@
 			// Open files for converting
 			foreach (var filePath in Directory.GetFiles(tempDirectory, "test-*.png", SearchOption.TopDirectoryOnly))
 			{
-				using (Image image = Image.NewFromBuffer(File.ReadAllBytes(filePath), access: Enums.Access.Sequential))
+				using (Image image = Image.NewFromFile(filePath, access: Enums.Access.Sequential))
 				{
 					// Do nothing, just read, this is already enough for issue
 					//image.Jpegsave(".....", 75);
@@ -67,9 +69,9 @@
 		// 10 iteration because the exception not stable
 		for (int i = 0; i < 10; i++)
 		{
-			// 20 parallel tasks for reading files via NetVips
+			// 30 parallel tasks for reading files via NetVips
 			var tasks = new List<Task>();
-			for (int j = 0; j < 20; j++)
+			for (int j = 0; j < 30; j++)
 				tasks.Add(DoAll($"TempDirectory-{i}-{j}"));
 
 			await Task.WhenAll(tasks);

[kleisauke]

Oh, I could still reproduce this after increasing it to 50 parallel tasks with the libvips binaries mentioned above. I added this in the exception handler:

catch (Exception e)
{
    var message = e.Message;

    var start = message.IndexOf("file '") + 6;
    var file = Path.Combine(tempDirectory, message[start..message.IndexOf('\'', start)]);

    Console.WriteLine($"{file} is locked by these processes:");
    foreach (var process in FileUtil.WhoIsLocking(file))
    {
        Console.WriteLine("\t" + process.ProcessName);
    }
    throw;
}

(FileUtil.WhoIsLocking is from https://stackoverflow.com/a/20623311)

This gives this information:

TempDirectory-5-4\test-19.png is locked by these processes:
        xcopy
System.IO.IOException: The process cannot access the file 'test-19.png' because it is being used by another process.
   at System.IO.FileSystem.RemoveDirectoryRecursive(String fullPath, WIN32_FIND_DATA& findData, Boolean topLevel)
   at System.IO.FileSystem.RemoveDirectory(String fullPath, Boolean recursive)

So, it looks like there's still something wrong with that xcopy process, let me investigate.

[kleisauke]

Here's a simplified test program that doesn't use xcopy. This seems to work for me without any problems.

Test program

namespace ConvertExample
{
    using System;
    using System.Diagnostics;
    using System.IO;
    using System.Threading;
    using System.Threading.Tasks;
    using NetVips;

    class Program
    {
        static void Convert(DirectoryInfo source, DirectoryInfo target)
        {
            // Check whether we are in different threads
            Debug.WriteLine($"Work with {target.FullName} in tid {Thread.CurrentThread.ManagedThreadId}");

            // Create target directory
            Directory.CreateDirectory(target.FullName);

            // Copy each image into the new directory
            foreach (var fi in source.GetFiles())
            {
                fi.CopyTo(Path.Combine(target.FullName, fi.Name), true);
            }

            // Convert images to a new format
            foreach (var fi in target.GetFiles())
            {
                using (var image = Image.NewFromFile(fi.FullName, access: Enums.Access.Sequential))
                {
                    // TODO: Image converting logic here
                }

                // We're done with the image, delete it
                fi.Delete();
            }

            // Delete the empty directory
            target.Delete();
        }

        static void Main(string[] args)
        {
            Cache.Max = 0;

            var basePath = @"C:\Temp\TestNetVips";
            var tempPath = Path.GetTempPath();

            var testImage = Path.Combine(basePath, "test.png");
            var baseDirectory = Path.Combine(basePath, "TestDirectory");

            // The base directory that we clone each time
            var source = new DirectoryInfo(baseDirectory);
            Directory.CreateDirectory(source.FullName);

            // Prepare 50 test images
            for (var i = 0; i < 50; i++)
                File.Copy(testImage, Path.Combine(source.FullName, $"test-{i}.png"), true);

            // Run as many parallel jobs as there are available CPU cores
            Parallel.For(0, 1000, new ParallelOptions { MaxDegreeOfParallelism = NetVips.Concurrency },
            i =>
            {
                var target = new DirectoryInfo(Path.Combine(tempPath, $"TempDirectory-{i}"));
                Convert(source, target);
            });

            Console.WriteLine("Done!");
        }
    }
}

[AndrePScope]

Hi @kleisauke !

Yes, your last code example works well.

But... I wrote the same in my initial post in this discussion:

So for the reproducing, needs 3 things:

1. creates files in the async method via System.Diagnostics.Process in some temp directory
2. read files via Image.NewFromFile
3. try to delete the temp directory

The problem is not in xcopy, but in System.Diagnostics.Process.
When we use an external process, we have an issue.

My code contains xcopy but it is only for example.
The real production code does not contain xcopy, but it contains another external tool that we have to call via System.Diagnostics.Process ( xpdf tools that generate images from pdf that we process by NetVips after).

[kleisauke]

I just found an interesting thread on Twitter: https://twitter.com/MagickNET/status/1451276675396816896
Relevant repository: https://github.com/dlemstra/FileLockingDemo

I can confirm that I was unable to reproduce this using UseShellExecute = true. So, it looks like some kind of .NET issue.

[kleisauke]

PR libvips/libvips#2497 should fix this. The pre-compiled libvips Windows binaries build from that PR can be downloaded here (for testing purposes):
https://libvips-packaging.s3.amazonaws.com/vips-dev-w64-web-8.12.0-29fb557-static.zip

[AndrePScope]

Hi @kleisauke !

Thank you very much for the solution!
I have tested and seen the fix libvips/libvips#2497 solves the problem!

As I understand for the production we will wait for the new version NetVips.Native 8.12.
And this version will contain the fix?

[kleisauke]

This fix will be in the future libvips 8.12. For now, you can use the pre-built binaries mentioned above. Another possible way to fix this is to serialize the calls to Process.Start, for example:

private static readonly Object obj = new Object();

lock (obj)
{
    using var proc = Process.Start( ...
}

(untested)

Or by using the buffer/stream-based loaders, as you noticed.

[kleisauke]

A release candidate of NetVips.Native v8.12.0 is now available for testing.
https://www.nuget.org/packages/NetVips.Native/8.12.0-rc1

[kleisauke]

NetVips.Native v8.12.1 is now available.

[AndrePScope]

Thank you very much!