Deadlock when enumerating resources with ResourceManager

[madelson]

Description

Enumerating resources with ResourceManager.GetResourceSet while concurrently calling ResourceManager.GetString can result in a hang which appears to be due to deadlock.

Reproduction Steps

The following code snippet hangs with high probability on .NET 6. Most commonly, this happens after printing the first entry on each thread.

void Main()
{
	var resourceType = typeof(MyResource); // standard resx with 15 entries
	var resourceManager = new ResourceManager(resourceType.FullName, resourceType.Assembly);
	
	var threads = 10;
	using var barrier = new Barrier(threads);
	Task.WaitAll(
		Enumerable.Range(0, threads)
			.Select(_ => Task.Run(() =>
			{
				barrier.SignalAndWait();
				EnumerateResources();
			}))
			.ToArray()
	); // never returns
	
	void EnumerateResources()
	{
		var set = resourceManager.GetResourceSet(CultureInfo.InvariantCulture, createIfNotExists: true, tryParents: true);
		foreach (var entry in set.Cast<DictionaryEntry>())
		{
			Console.WriteLine(resourceManager.GetString((string)entry.Key));
			Thread.Sleep(1);
		}
	}
}

Expected behavior

ResourceManager is supposed to be thread safe, so this code should work.

Actual behavior

Frequent hangs.

Regression?

I know that this used to be broken in some early versions of .NET Framework as well and was fixed at some point (perhaps in one of the 4.6 releases?). Testing the same snippet on .NET Framework latest does not hang, so I believe this is a regression.

EDIT: based on further testing this appears to be a regression vs. .NET Core 3.1 as well.

Known Workarounds

Allocate a new ResourceManager instance when planning to call GetResourceSet rather than re-using a shared instance.

Another sort-of workaround is to use the Key property on the enumerator instead of the Current property. This avoids the deadlock and makes more progress, but we instead run into #74052 .

Configuration

.NET 6 (6.0.5) Windows 10 x64.

Other information

In RuntimeResourceSet, we lock the cache first and then lock the ResourceReader.

In ResourceReader.ResourceEnumerator, we lock the reader first and then the cache.

I believe that this is the cause of the deadlock.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-resources
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Enumerating resources with ResourceManager.GetResourceSet while concurrently calling ResourceManager.GetString can result in a hang which appears to be due to deadlock.

Reproduction Steps

The following code snippet hangs with high probability on .NET 6. Most commonly, this happens after printing the first entry on each thread.

void Main()
{
	var resourceType = typeof(MyResource); // standard resx with 15 entries
	var resourceManager = new ResourceManager(resourceType.FullName, resourceType.Assembly);
	
	var threads = 10;
	using var barrier = new Barrier(threads);
	Task.WaitAll(
		Enumerable.Range(0, threads)
			.Select(_ => Task.Run(() =>
			{
				barrier.SignalAndWait();
				EnumerateResources();
			}))
			.ToArray()
	); // never returns
	
	void EnumerateResources()
	{
		var set = resourceManager.GetResourceSet(CultureInfo.InvariantCulture, createIfNotExists: true, tryParents: true);
		foreach (var entry in set.Cast<DictionaryEntry>())
		{
			Console.WriteLine(resourceManager.GetString((string)entry.Key));
			Thread.Sleep(1);
		}
	}
}

Expected behavior

ResourceManager is supposed to be thread safe, so this code should work.

Actual behavior

Frequent hangs.

Regression?

I know that this used to be broken in some early versions of .NET Framework as well and was fixed at some point (perhaps in one of the 4.6 releases?). Testing the same snippet on .NET Framework latest does not hang, so I believe this is a regression.

Known Workarounds

Allocate a new ResourceManager instance when planning to call GetResourceSet rather than re-using a shared instance.

Another sort-of workaround is to use the Key property on the enumerator instead of the Current property. This avoids the deadlock and makes more progress, but we instead run into #74052 .

Configuration

.NET 6 (6.0.5) Windows 10 x64.

Other information

In RuntimeResourceSet, we lock the cache first and then lock the ResourceReader.

In ResourceReader.ResourceEnumerator, we lock the reader first and then the cache.

I believe that this is the cause of the deadlock.

Author:	madelson
Assignees:	-
Labels:	area-System.Resources
Milestone:	-

[madelson]

I'd be happy to work on this as part of working on #74052

[joperezr]

Thanks for the report @madelson, and also for checking when did this regressed. From your original post though, I see that this issue was present in .NET 6 (meaning it wasn't a regression from .NET 6 to .NET 7), so I'm going to move this issue out of .NET 7 by setting the milestone to .NET 8 for now. Thanks again for working on this!

[madelson]

@joperezr it isn't a regression from 6->7, but it is a regression from the previous LTS release (3.1) to the current LTS release (6). I imagine that the bar is too high to consider backporting this to 6 but given the severity of the issue if you encounter it (deadlocking the app) and the fact that apps may be unprepared for this given its previous fix, it would be nice to at least consider that.

[joperezr]

That's right, I didn't mean that this shouldn't get fixed in servicing, sorry for not making that clear. What I meant was that the fix is likely not going to hit .NET 7 before we ship, so I just adjusted the milestone to vNext for now. The process to get this regression fixed in servicing would be to:

1. Fix the issue in main branch (which today means 8.0 and the reason why I picked that milestone)
2. Once the fix is ready and we have validated that the fix works as expected, we can then make a case for us to service this in all impacted releases (which would be 6.0, and 7.0). If approved, then we can cherry-pick the change from main into those servicing branches and create PRs for them in order to get the fix serviced.