System.Text.Json doesn't call IEnumerator<T>.Dispose

[LXGaming]

Description

I have a custom collection implementation which relies on IEnumerator<T>.Dispose being called in order to release a read lock, however I discovered that on serialization System.Text.Json will grab the IEnumerator<T> but does not appear to call Dispose which results in my custom collection being in a permanent ReadLocked state.

I've just switched from Newtonsoft.Json which doesn't have this issue.

Example Code: https://gist.github.com/LXGaming/862f498de548c67d0ed7d193d22d2499

Configuration

• Which version of .NET is the code running on? .NET 5
• What OS and version, and what distro if applicable? Windows 10 20H2, dotnet/runtime:5.0 Docker Image on Ubuntu 20.04
• What is the architecture (x64, x86, ARM, ARM64)? x64
• Do you know whether it is specific to that configuration? I don't believe so.
• If you're using Blazor, which web browser(s) do you see this issue in? N/A

Regression?

I don't know.

Other information

This issue can be worked around by creating a custom converter for the custom collection.

This issue is also present in several other Converters, The IEnumerator<T> is obtained in OnWriteResume

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ICollectionOfTConverter.cs

Lines 56 to 94 in 7360c19

	protected override bool OnWriteResume(
	Utf8JsonWriter writer,
	TCollection value,
	JsonSerializerOptions options,
	ref WriteStack state)
	{
	IEnumerator<TElement> enumerator;
	if (state.Current.CollectionEnumerator == null)
	{
	enumerator = value.GetEnumerator();
	if (!enumerator.MoveNext())
	{
	return true;
	}
	}
	else
	{
	enumerator = (IEnumerator<TElement>)state.Current.CollectionEnumerator;
	}
	JsonConverter<TElement> converter = GetElementConverter(ref state);
	do
	{
	if (ShouldFlush(writer, ref state))
	{
	state.Current.CollectionEnumerator = enumerator;
	return false;
	}
	TElement element = enumerator.Current;
	if (!converter.TryWrite(writer, element, options, ref state))
	{
	state.Current.CollectionEnumerator = enumerator;
	return false;
	}
	} while (enumerator.MoveNext());
	return true;
	}

[danmoseley]

There are 16 instances of value.GetEnumerator() where (if I read it correctly) we are not disposing it.

[GSPP]

This is important as well for sequences that read from a database or from the file system.

[layomia]

There are 16 instances of value.GetEnumerator() where (if I read it correctly) we are not disposing it.

Yes, thereabout - minus a couple of calls on IEnumerator.

Marking as up-for-grabs - we should do this for correctness. It would be good to note any perf impact to serializing built-in collections by running existing benchmarks .