Skip to content

System.Text.Json not properly disposing enumerators on exceptions #50851

Closed

Description

@eiriktsarpalis

Related to #48322 and #46349.

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.

Activity

  1. added this to the 6.0.0 milestone on Apr 7, 2021
  2. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Apr 7, 2021
  3. removed
    untriagedNew issue has not been triaged by the area owner
    on Apr 7, 2021
  4. added
    enhancementProduct code improvement that does NOT require public API changes/additions
    on Jul 30, 2021
  5. steveharter commented on Aug 4, 2021

    @steveharter
    Contributor

    Suggest moving to 7.0. No community asks that I'm aware of.

  6. modified the milestones: 6.0.0, 7.0.0 on Aug 9, 2021
  7. eiriktsarpalis commented on Aug 16, 2021

    @eiriktsarpalis
    MemberAuthor

    Note that this should be fixed for Dictionary converters as well.

  8. modified the milestones: 7.0.0, Future on Apr 18, 2022
  9. eiriktsarpalis commented on Apr 18, 2022

    @eiriktsarpalis
    MemberAuthor

    Moving to Future, as we won't have time to work on this in the .NET 7 timeframe.

  10. added
    help wanted[up-for-grabs] Good issue for external contributors
    on Dec 2, 2022
  11. added
    in-prThere is an active PR which will close this issue when it is merged
    on Mar 23, 2024
  12. eiriktsarpalis commented on Jun 24, 2024

    @eiriktsarpalis
    MemberAuthor

    After deliberation we've decided to not fix this bug. It is largely a theoretical concern that no user has reported so far.

  13. locked and limited conversation to collaborators on Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

area-System.Text.JsonenhancementProduct code improvement that does NOT require public API changes/additionshelp wanted[up-for-grabs] Good issue for external contributorsin-prThere is an active PR which will close this issue when it is merged

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions