Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix json converters not disposing enumerators #48322

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand All @@ -58,6 +59,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
}
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand All @@ -58,6 +59,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
}
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ protected internal override bool OnWriteResume(
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand Down Expand Up @@ -93,6 +94,7 @@ protected internal override bool OnWriteResume(
} while (enumerator.MoveNext());
}

enumerator.Dispose();
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ protected override bool OnWriteResume(
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand All @@ -90,6 +91,7 @@ protected override bool OnWriteResume(
}
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ protected internal override bool OnWriteResume(
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand Down Expand Up @@ -102,6 +103,7 @@ protected internal override bool OnWriteResume(
state.Current.EndDictionaryElement();
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand All @@ -62,6 +63,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
}
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand All @@ -86,6 +87,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
}
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ protected internal override bool OnWriteResume(Utf8JsonWriter writer, TCollectio
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand Down Expand Up @@ -69,6 +70,7 @@ protected internal override bool OnWriteResume(Utf8JsonWriter writer, TCollectio
state.Current.EndDictionaryElement();
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand All @@ -83,6 +84,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
}
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ protected internal override bool OnWriteResume(Utf8JsonWriter writer, TCollectio
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand Down Expand Up @@ -80,6 +81,7 @@ protected internal override bool OnWriteResume(Utf8JsonWriter writer, TCollectio
state.Current.EndDictionaryElement();
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand All @@ -68,6 +69,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
}
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand All @@ -57,6 +58,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
}
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
enumerator = value.GetEnumerator();
if (!enumerator.MoveNext())
{
enumerator.Dispose();
return true;
}
}
Expand All @@ -57,6 +58,7 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
}
} while (enumerator.MoveNext());

enumerator.Dispose();
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,78 @@ public static void ConvertIEnumerableValueTypesThenSerialize()
Assert.Equal(expectedJson, JsonSerializer.Serialize<IEnumerable<ValueB>>(valueBs));
}

[Fact]
public static void WriteIEnumerableT_DisposesEnumerators()
{
for (int count = 0; count < 5; count++)
{
var items = new RefCountedList<int>(Enumerable.Range(1, count));
_ = JsonSerializer.Serialize(items.AsEnumerable());

Assert.Equal(0, items.RefCount);
}
}

[Fact]
public static void WriteICollectionT_DisposesEnumerators()
{
for (int count = 0; count < 5; count++)
{
var items = new RefCountedList<int>(Enumerable.Range(1, count));
_ = JsonSerializer.Serialize((ICollection<int>)items);

Assert.Equal(0, items.RefCount);
}
}

[Fact]
public static void WriteIListT_DisposesEnumerators()
{
for (int count = 0; count < 5; count++)
{
var items = new RefCountedList<int>(Enumerable.Range(1, count));
_ = JsonSerializer.Serialize((IList<int>)items);

Assert.Equal(0, items.RefCount);
}
}

[Fact]
public static void WriteIDictionaryT_DisposesEnumerators()
{
for (int count = 0; count < 5; count++)
{
var pairs = new RefCountedDictionary<int, int>(Enumerable.Range(1, count).Select(x => new KeyValuePair<int, int>(x, x)));
_ = JsonSerializer.Serialize((IDictionary<int, int>)pairs);

Assert.Equal(0, pairs.RefCount);
}
}

[Fact]
public static void WriteIReadOnlyDictionaryT_DisposesEnumerators()
{
for (int count = 0; count < 5; count++)
{
var pairs = new RefCountedDictionary<int, int>(Enumerable.Range(1, count).Select(x => new KeyValuePair<int, int>(x, x)));
_ = JsonSerializer.Serialize((IReadOnlyDictionary<int, int>)pairs);

Assert.Equal(0, pairs.RefCount);
}
}

[Fact]
public static void WriteISetT_DisposesEnumerators()
{
for (int count = 0; count < 5; count++)
{
var items = new RefCountedSet<int>(Enumerable.Range(1, count));
_ = JsonSerializer.Serialize((ISet<int>)items);

Assert.Equal(0, items.RefCount);
}
}

public class SimpleClassWithKeyValuePairs
{
public KeyValuePair<string, string> KvpWStrVal { get; set; }
Expand All @@ -901,5 +973,57 @@ public class ValueB
{
public int Value { get; set; }
}

private class RefCountedList<T> : List<T>, IEnumerable<T> // Reimplement interface.
{
public RefCountedList() : base() { }
public RefCountedList(IEnumerable<T> collection) : base(collection) { }

public int RefCount { get; private set; }

IEnumerator<T> IEnumerable<T>.GetEnumerator()
{
RefCount++;
return new DisposableEnumerator<T>(GetEnumerator(), () => RefCount--);
}

Collections.IEnumerator Collections.IEnumerable.GetEnumerator() => this.AsEnumerable().GetEnumerator();
}

private class RefCountedDictionary<TKey, TValue> : Dictionary<TKey, TValue>, IEnumerable<KeyValuePair<TKey, TValue>> // Reimplement interface.
{
public RefCountedDictionary() : base() { }
public RefCountedDictionary(IEnumerable<KeyValuePair<TKey, TValue>> collection)
{
foreach (var kvp in collection)
Add(kvp.Key, kvp.Value);
}

public int RefCount { get; private set; }

IEnumerator<KeyValuePair<TKey, TValue>> IEnumerable<KeyValuePair<TKey, TValue>>.GetEnumerator()
{
RefCount++;
return new DisposableEnumerator<KeyValuePair<TKey, TValue>>(GetEnumerator(), () => RefCount--);
}

Collections.IEnumerator Collections.IEnumerable.GetEnumerator() => this.AsEnumerable().GetEnumerator();
}

private class RefCountedSet<T> : HashSet<T>, IEnumerable<T> // Reimplement interface.
{
public RefCountedSet() : base() { }
public RefCountedSet(IEnumerable<T> collection) : base(collection) { }

public int RefCount { get; private set; }

IEnumerator<T> IEnumerable<T>.GetEnumerator()
{
RefCount++;
return new DisposableEnumerator<T>(GetEnumerator(), () => RefCount--);
}

Collections.IEnumerator Collections.IEnumerable.GetEnumerator() => this.AsEnumerable().GetEnumerator();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1641,4 +1641,30 @@ public void Verify()
Assert.Equal("value1", Dictionary.Value["key1"]);
}
}

public class DisposableEnumerator<T> : IEnumerator<T>
{
private readonly IEnumerator<T> _inner;
private Action _onDispose;

public DisposableEnumerator(IEnumerator<T> inner, Action onDispose)
{
_inner = inner;
_onDispose = onDispose;
}

public T Current => _inner.Current;

object IEnumerator.Current => ((IEnumerator)_inner).Current;

public bool MoveNext() => _inner.MoveNext();

public void Dispose()
{
_onDispose?.Invoke();
_onDispose = null;
}

public void Reset() => _inner.Reset();
}
}