-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Closed
Labels
area-System.Text.JsonenhancementProduct code improvement that does NOT require public API changes/additionsProduct code improvement that does NOT require public API changes/additionshelp wanted[up-for-grabs] Good issue for external contributors[up-for-grabs] Good issue for external contributorsin-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged
Milestone
Description
The current IEnumerable converters don't dispose of enumerators in certain scenaria where nested element converters throw exceptions. Reproducing test (using a few System.Text.Json.Tests components):
[Fact]
public static void WriteIEnumerableT_ElementSerializationThrows_DisposesEnumerators()
{
var items = new RefCountedList<IEnumerable<int>>(Enumerable.Repeat(ThrowingEnumerable(), 1));
Assert.Throws<DivideByZeroException>(() => JsonSerializer.Serialize(items.AsEnumerable()));
Assert.Equal(0, items.RefCount); // items.RefCount evaluates to 1
static IEnumerable<int> ThrowingEnumerable()
{
yield return 42;
throw new DivideByZeroException();
}
}Note that #50778 makes changes to the serialization infrastructure to handle IEnumerator disposal in the async case, however changes on the individual converter level will still need to be made. cc @layomia @steveharter.
Metadata
Metadata
Assignees
Labels
area-System.Text.JsonenhancementProduct code improvement that does NOT require public API changes/additionsProduct code improvement that does NOT require public API changes/additionshelp wanted[up-for-grabs] Good issue for external contributors[up-for-grabs] Good issue for external contributorsin-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged